On Thu, 2 Jan 2025 at 14:53, Yongbang Shi <[email protected]> wrote:
>
> > On Thu, Dec 19, 2024 at 10:52:32AM +0800, Yongbang Shi wrote:
> >> Does everyone have a question with the patch?
> > -:225: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'reg_value' - possible
> > side-effects?
> > #225: FILE: drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h:23:
> > +#define dp_field_modify(reg_value, mask, val)
> > \
> > + do { \
> > + (reg_value) &= ~(mask); \
> > + (reg_value) |= FIELD_PREP(mask, val); \
> > + } while (0) \
> > +
> >
> > -:225: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'mask' - possible
> > side-effects?
> > #225: FILE: drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h:23:
> > +#define dp_field_modify(reg_value, mask, val)
> > \
> > + do { \
> > + (reg_value) &= ~(mask); \
> > + (reg_value) |= FIELD_PREP(mask, val); \
> > + } while (0) \
> > +
>
> Hi Dmitry,
> Thanks for your reminding. For this point, we have to use two variables(mask,
> reg_value) twice here.
> I tried to another way that static inline functions, but there are build
> errors. So the only way is like this.
> After reviewed all references, we have a review session about it, and we
> think it's acceptable and relatively safe.
> (like we won't use varible with post/pre-increment operator)
> If you have a better way to realize this, plz tell me. Appreciate!
Well, there is an obvious way:
do {
u32 __mask = (mask);
(reg_value) &= ~__mask;
(reg_value) |= FIELD_PREP(__mask, (val));
} while (0)
But I don't really think that it makes sense to change. It's a
check-level, not a warning. The next one is important.
>
>
> > -:277: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name
> > mismatch: 'From: baihan li <[email protected]>' != 'Signed-off-by: Baihan
> > Li <[email protected]>'
> >
> > total: 0 errors, 2 warnings, 2 checks, 239 lines checked
> > 1c3faaf4e729 drm/hisilicon/hibmc: add dp link moduel in hibmc drivers
> > -:70: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does
> > MAINTAINERS need updating?
> > #70:
> > new file mode 100644
> >
> > -:435: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name
> > mismatch: 'From: baihan li <[email protected]>' != 'Signed-off-by: Baihan
> > Li <[email protected]>'
> >
> > total: 0 errors, 2 warnings, 0 checks, 399 lines checked
> > 70bf7b765c82 drm/hisilicon/hibmc: add dp hw moduel in hibmc driver
> > -:28: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does
> > MAINTAINERS need updating?
> > #28:
> > new file mode 100644
> >
> > -:371: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name
> > mismatch: 'From: baihan li <[email protected]>' != 'Signed-off-by: Baihan
> > Li <[email protected]>'
> >
> > total: 0 errors, 2 warnings, 0 checks, 327 lines checked
> > 24d6be2577d8 drm/hisilicon/hibmc: refactored struct hibmc_drm_private
> > -:188: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name
> > mismatch: 'From: baihan li <[email protected]>' != 'Signed-off-by: Baihan
> > Li <[email protected]>'
> >
> > total: 0 errors, 1 warnings, 0 checks, 151 lines checked
> > 9a74395d14e2 (HEAD -> drm-misc-next) drm/hisilicon/hibmc: add dp module in
> > hibmc
> > -:30: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does
> > MAINTAINERS need updating?
> > #30:
> > new file mode 100644
> >
> > -:219: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name
> > mismatch: 'From: baihan li <[email protected]>' != 'Signed-off-by: Baihan
> > Li <[email protected]>'
>
> We already have maintainers for our hibmc-drm module, so there is no need add
> maintainers in each new file.
Please read carefully. Those warnings are about sign-off mismatches.
You can ignore the maintainers entry.
>
> Thanks ,
> Baihan Li
>
>
> >>
> >>> From: baihan li <[email protected]>
> >>>
> >>> Realizing the basic display function of DP cable for DP connector
> >>> displaying. Add DP module in hibmc drm driver, which is for Hisilicon
> >>> Hibmc SoC which used for Out-of-band management. Blow is the general
> >>> hardware connection, both the Hibmc and the host CPU are on the same
> >>> mother board.
> >>>
> >>> +----------+ +----------+ +----- ----+ +----------------+
> >>> | | PCIe | Hibmc | | | | |
> >>> |host CPU( |<----->| display |<---->| dp kapi |<---->| dp aux moduel |
> >>> |arm64,x86)| |subsystem | | moduel |<---->| dp link moduel |
> >>> +----------+ +----------+ +----------+ +----------------+
> >>>
> >>> ---
> >>> ChangeLog:
> >>> v7 -> v8:
> >>> - adding DP_AUX_NATIVE_WRITE case in switch statement in
> >>> hibmc_dp_aux_parse_xfer().
> >>> - adding pci_set_master() in hibmc_pci_probe().
> >>>
> >>> v7:https://lore.kernel.org/all/[email protected]/
> >>> v6 -> v7:
> >>> - deleteing unset fields in struct hibmc_link_cap, suggested by
> >>> Dmitry Baryshkov.
> >>> - using macro instead of constants in
> >>> hibmc_dp_link_training_configure(), suggested by Dmitry Baryshkov.
> >>> - lowercasing hex, suggested by Dmitry Baryshkov.
> >>>
> >>> v6:https://lore.kernel.org/all/[email protected]/
> >>> v5 -> v6:
> >>> - adding do{} while(0) in macro defination function, suggested by
> >>> Dmitry Baryshkov.
> >>> - using drm_dbg_dp() to print debug info instead of drm_info(),
> >>> suggested by Dmitry Baryshkov.
> >>> - adding code comments in hibmc_dp_set_sst(), suggested by Dmitry
> >>> Baryshkov.
> >>> - adding hpd and get_edid comments in the beginning of patch,
> >>> suggested by Dmitry Baryshkov.
> >>>
> >>> v5:https://lore.kernel.org/all/[email protected]/
> >>> v4 -> v5:
> >>> - fixing build errors reported by kernel test robot <[email protected]>
> >>> Closes:
> >>> https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >>>
> >>> v4:https://lore.kernel.org/all/[email protected]/
> >>> v3 -> v4:
> >>> - retun error codes in result incorrect branch, suggested by Dmitry
> >>> Baryshkov.
> >>> - replacing all ret= with returns, suggested by Dmitry Baryshkov.
> >>> - moving the comment below the judgment statement, suggested by
> >>> Dmitry Baryshkov.
> >>> - moving definations to the source file and clearing headers,
> >>> suggested by Dmitry Baryshkov.
> >>> - reanaming dp_prefix to hibmc_dp_prefix, suggested by Dmitry
> >>> Baryshkov.
> >>> - changing hibmc_dp_reg_write_field to static inline and lock,
> >>> suggested by Dmitry Baryshkov.
> >>> - moving some structs to later patch, suggested by Dmitry Baryshkov.
> >>> - optimizing hibmc_dp_link_get_adjust_train() to delete for loop,
> >>> suggested by Dmitry Baryshkov.
> >>> - changing ELNRNG to EIO error code, suggested by Dmitry Baryshkov.
> >>> - deleting meaningless macro, suggested by Dmitry Baryshkov.
> >>> - fixing build errors reported by kernel test robot <[email protected]>
> >>> Closes:
> >>> https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >>> - changed the type of train_set to array, suggested by Dmitry
> >>> Baryshkov.
> >>> - using actual link rate instead of magic num, suggested by Dmitry
> >>> Baryshkov.
> >>> - deleting hibmc_dp_hw_uninit(), suggested by Dmitry Baryshkov.
> >>> - separating hibmc_vdac and hibmc_dp changes into separate patche,
> >>> suggested by Dmitry Baryshkov.
> >>> - static int hibmc_dp_prepare(), suggested by Dmitry Baryshkov.
> >>>
> >>> v3:https://lore.kernel.org/all/[email protected]/
> >>> v2 -> v3:
> >>> - put the macro definations in latter patch where they are actually
> >>> used, suggested by Dmitry Baryshkov.
> >>> - rename some macro definations to make them sensible, suggested by
> >>> Dmitry Baryshkov.
> >>> - using FIELD_PREP and FIELD_GET, suggested by Dmitry Baryshkov.
> >>> - using DP_DPCD_REV_foo, suggested by Dmitry Baryshkov.
> >>> - using switchcase in dp_link_reduce_lane, suggested by Dmitry
> >>> Baryshkov.
> >>> - deleting dp_link_pattern2dpcd function and using macros directly,
> >>> suggested by Dmitry Baryshkov.
> >>> - deleting EFAULT error codes, suggested by Dmitry Baryshkov.
> >>> - fix build errors reported by kernel test robot <[email protected]>
> >>> Closes:
> >>> https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >>> Closes:
> >>> https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >>> Closes:
> >>> https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >>>
> >>> v2:https://lore.kernel.org/all/[email protected]/
> >>> v1 -> v2:
> >>> - using drm_dp_aux frame implement dp aux read and write functions,
> >>> suggested by Jani Nikula.
> >>> - using drm dp header files' dp macros instead, suggested by Andy Yan.
> >>> - using drm_dp_* functions implement dp link training process,
> >>> suggested by Jani Nikula.
> >>> - changed some defines and functions to former patch, suggested by
> >>> Dmitry Baryshkov.
> >>> - sorting the headers including in dp_hw.h and hibmc_drm_drv.c files,
> >>> suggested by Dmitry Baryshkov.
> >>> - deleting struct dp_mode and dp_mode_cfg function, suggested by
> >>> Dmitry Baryshkov.
> >>> - modifying drm_simple_encoder_init function, suggested by Dmitry
> >>> Baryshkov.
> >>> - refactoring struct hibmc_connector, suggested by Dmitry Baryshkov.
> >>> - withdrawing the modification in hibmc_kms_init, suggested by Dmitry
> >>> Baryshkov.
> >>> - fix build errors reported by kernel test robot <[email protected]>
> >>> Closes:
> >>> https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >>> Closes:
> >>> https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >>>
> >>> v1:https://lore.kernel.org/all/[email protected]/
> >>> ---
> >>>
> >>> baihan li (5):
> >>> drm/hisilicon/hibmc: add dp aux in hibmc drivers
> >>> drm/hisilicon/hibmc: add dp link moduel in hibmc drivers
> >>> drm/hisilicon/hibmc: add dp hw moduel in hibmc driver
> >>> drm/hisilicon/hibmc: refactored struct hibmc_drm_private
> >>> drm/hisilicon/hibmc: add dp module in hibmc
> >>>
> >>> drivers/gpu/drm/hisilicon/hibmc/Makefile | 3 +-
> >>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c | 164 +++++++++
> >>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h | 63 ++++
> >>> .../gpu/drm/hisilicon/hibmc/dp/dp_config.h | 19 +
> >>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 220 ++++++++++++
> >>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 28 ++
> >>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c | 332 ++++++++++++++++++
> >>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h | 76 ++++
> >>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 118 +++++++
> >>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 14 +
> >>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 19 +-
> >>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c | 41 ++-
> >>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 20 +-
> >>> 13 files changed, 1078 insertions(+), 39 deletions(-)
> >>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
> >>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
> >>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
> >>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> >>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> >>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
> >>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> >>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> >>>
--
With best wishes
Dmitry
