Re: [PATCH v2 03/12] drm/i915: Make I2C terminology more inclusive
On Fri, May 03, 2024 at 06:13:24PM +, Easwar Hariharan wrote:
> I2C v7, SMBus 3.2, and I3C 1.1.1 specifications have replaced "master/slave"
> with more appropriate terms. Inspired by and following on to Wolfram's
> series to fix drivers/i2c/[1], fix the terminology for users of
> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists
> in the specification.
>
> Compile tested, no functionality changes intended
>
> [1]:
> https://lore.kernel.org/all/20240322132619.6389-1-wsa+rene...@sang-engineering.com/
>
> Reviewed-by: Rodrigo Vivi
> Acked-by: Rodrigo Vivi
It looks like the ack is not needed since we are merging this through
drm-intel-next. But I'm planing to merge this only after seeing the
main drivers/i2c accepting the new terminology. So we don't have a
risk of that getting push back and new names there and we having
to rename it once again.
(more below)
> Acked-by: Zhi Wang
> Signed-off-by: Easwar Hariharan
Cc: Jani Nikula
Jani, what bits were you concerned that were not necessarily i2c?
I believe although not necessarily/directly i2c, I believe they
are related and could benefit from the massive single shot renable.
or do you have any better split to suggest here?
(more below)
> ---
> drivers/gpu/drm/i915/display/dvo_ch7017.c | 14 -
> drivers/gpu/drm/i915/display/dvo_ch7xxx.c | 18 +--
> drivers/gpu/drm/i915/display/dvo_ivch.c | 16 +-
> drivers/gpu/drm/i915/display/dvo_ns2501.c | 18 +--
> drivers/gpu/drm/i915/display/dvo_sil164.c | 18 +--
> drivers/gpu/drm/i915/display/dvo_tfp410.c | 18 +--
> drivers/gpu/drm/i915/display/intel_bios.c | 22 +++---
> drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
> .../gpu/drm/i915/display/intel_display_core.h | 2 +-
> drivers/gpu/drm/i915/display/intel_dsi.h | 2 +-
> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 20 ++---
> drivers/gpu/drm/i915/display/intel_dvo.c | 14 -
> drivers/gpu/drm/i915/display/intel_dvo_dev.h | 2 +-
> drivers/gpu/drm/i915/display/intel_gmbus.c| 4 +--
> drivers/gpu/drm/i915/display/intel_sdvo.c | 30 +--
> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 4 +--
> drivers/gpu/drm/i915/gvt/edid.c | 28 -
> drivers/gpu/drm/i915/gvt/edid.h | 4 +--
> drivers/gpu/drm/i915/gvt/opregion.c | 2 +-
> 19 files changed, 119 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/dvo_ch7017.c
> b/drivers/gpu/drm/i915/display/dvo_ch7017.c
> index d0c3880d7f80..493e730c685b 100644
> --- a/drivers/gpu/drm/i915/display/dvo_ch7017.c
> +++ b/drivers/gpu/drm/i915/display/dvo_ch7017.c
> @@ -170,13 +170,13 @@ static bool ch7017_read(struct intel_dvo_device *dvo,
> u8 addr, u8 *val)
> {
> struct i2c_msg msgs[] = {
> {
> - .addr = dvo->slave_addr,
> + .addr = dvo->target_addr,
> .flags = 0,
> .len = 1,
> .buf = &addr,
> },
> {
> - .addr = dvo->slave_addr,
> + .addr = dvo->target_addr,
> .flags = I2C_M_RD,
> .len = 1,
> .buf = val,
> @@ -189,7 +189,7 @@ static bool ch7017_write(struct intel_dvo_device *dvo, u8
> addr, u8 val)
> {
> u8 buf[2] = { addr, val };
> struct i2c_msg msg = {
> - .addr = dvo->slave_addr,
> + .addr = dvo->target_addr,
> .flags = 0,
> .len = 2,
> .buf = buf,
> @@ -197,7 +197,7 @@ static bool ch7017_write(struct intel_dvo_device *dvo, u8
> addr, u8 val)
> return i2c_transfer(dvo->i2c_bus, &msg, 1) == 1;
> }
>
> -/** Probes for a CH7017 on the given bus and slave address. */
> +/** Probes for a CH7017 on the given bus and target address. */
> static bool ch7017_init(struct intel_dvo_device *dvo,
> struct i2c_adapter *adapter)
> {
> @@ -227,13 +227,13 @@ static bool ch7017_init(struct intel_dvo_device *dvo,
> break;
> default:
> DRM_DEBUG_KMS("ch701x not detected, got %d: from %s "
> - "slave %d.\n",
> - val, adapter->name, dvo->slave_addr);
> + "target %d.\n",
> + val, adapter->name, dvo->target_addr);
> goto fail;
> }
>
> DRM_DEBUG_KMS("%s detected on %s, addr %d\n",
> - str, adapter->name, dvo->slave_addr);
> + str, adapter->name, dvo->target_addr);
> return true;
>
> fail:
> diff --git a/drivers/gpu/drm/i915/display/dvo_ch7xxx.c
> b/drivers/gpu/drm/i915/display/dvo_ch7xxx.c
> index 2e8e85da5a40..534b8544e0a4 100644
> --- a/drivers/gpu/drm/i915/display/dvo_ch7xxx.c
> +++ b/drivers/gpu/drm/i915/
Re: [PATCH v2 03/12] drm/i915: Make I2C terminology more inclusive
On Fri, May 03, 2024 at 02:04:15PM -0700, Easwar Hariharan wrote: > On 5/3/2024 12:34 PM, Rodrigo Vivi wrote: > > On Fri, May 03, 2024 at 06:13:24PM +, Easwar Hariharan wrote: > >> I2C v7, SMBus 3.2, and I3C 1.1.1 specifications have replaced > >> "master/slave" > >> with more appropriate terms. Inspired by and following on to Wolfram's > >> series to fix drivers/i2c/[1], fix the terminology for users of > >> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists > >> in the specification. > >> > >> Compile tested, no functionality changes intended > >> > >> [1]: > >> https://lore.kernel.org/all/20240322132619.6389-1-wsa+rene...@sang-engineering.com/ > >> > >> Reviewed-by: Rodrigo Vivi > >> Acked-by: Rodrigo Vivi > > > > It looks like the ack is not needed since we are merging this through > > drm-intel-next. But I'm planing to merge this only after seeing the > > main drivers/i2c accepting the new terminology. So we don't have a > > risk of that getting push back and new names there and we having > > to rename it once again. > > Just to be explicit, did you want me to remove the Acked-by in v3, or will > you when you pull > the patch into drm-intel-next? > > > > > (more below) > > > >> Acked-by: Zhi Wang > >> Signed-off-by: Easwar Hariharan > > > > Cc: Jani Nikula > > > > Jani, what bits were you concerned that were not necessarily i2c? > > I believe although not necessarily/directly i2c, I believe they > > are related and could benefit from the massive single shot renable. > > or do you have any better split to suggest here? > > > > (more below) > > > >> --- > >> drivers/gpu/drm/i915/display/dvo_ch7017.c | 14 - > >> drivers/gpu/drm/i915/display/dvo_ch7xxx.c | 18 +-- > >> drivers/gpu/drm/i915/display/dvo_ivch.c | 16 +- > >> drivers/gpu/drm/i915/display/dvo_ns2501.c | 18 +-- > >> drivers/gpu/drm/i915/display/dvo_sil164.c | 18 +-- > >> drivers/gpu/drm/i915/display/dvo_tfp410.c | 18 +-- > >> drivers/gpu/drm/i915/display/intel_bios.c | 22 +++--- > >> drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- > >> .../gpu/drm/i915/display/intel_display_core.h | 2 +- > >> drivers/gpu/drm/i915/display/intel_dsi.h | 2 +- > >> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 20 ++--- > >> drivers/gpu/drm/i915/display/intel_dvo.c | 14 - > >> drivers/gpu/drm/i915/display/intel_dvo_dev.h | 2 +- > >> drivers/gpu/drm/i915/display/intel_gmbus.c| 4 +-- > >> drivers/gpu/drm/i915/display/intel_sdvo.c | 30 +-- > >> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 4 +-- > >> drivers/gpu/drm/i915/gvt/edid.c | 28 - > >> drivers/gpu/drm/i915/gvt/edid.h | 4 +-- > >> drivers/gpu/drm/i915/gvt/opregion.c | 2 +- > >> 19 files changed, 119 insertions(+), 119 deletions(-) > >> > > > > >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > >> b/drivers/gpu/drm/i915/display/intel_ddi.c > >> index c17462b4c2ac..64db211148a8 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c > >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > >> @@ -4332,7 +4332,7 @@ static int intel_ddi_compute_config_late(struct > >> intel_encoder *encoder, > >> > >> connector->tile_group->id); > >> > >>/* > >> - * EDP Transcoders cannot be ensalved > >> + * EDP Transcoders cannot be slaves > > > > ^ here > > perhaps you meant 'targeted' ? > > > >> * make them a master always when present > > > > This is not actually I2C related as far as I could tell when I was making the > change, so this was more of a typo fix. > > If we want to improve this, a quick check with the eDP v1.5a spec suggests > using primary/secondary instead, > though in a global fashion rather than specifically for eDP transcoders. > There is also source/sink terminology > in the spec related to DP encoders. > > Which would be a more acceptable change here? hmmm probably better to split the patches and align with the spec naming where it applies. and with i2c name where it applies. > > Thanks, > Easwar
Re: [PATCH v2 03/12] drm/i915: Make I2C terminology more inclusive
On Fri, 03 May 2024, Rodrigo Vivi wrote: > On Fri, May 03, 2024 at 02:04:15PM -0700, Easwar Hariharan wrote: >> On 5/3/2024 12:34 PM, Rodrigo Vivi wrote: >> > On Fri, May 03, 2024 at 06:13:24PM +, Easwar Hariharan wrote: >> >> I2C v7, SMBus 3.2, and I3C 1.1.1 specifications have replaced >> >> "master/slave" >> >> with more appropriate terms. Inspired by and following on to Wolfram's >> >> series to fix drivers/i2c/[1], fix the terminology for users of >> >> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists >> >> in the specification. >> >> >> >> Compile tested, no functionality changes intended >> >> >> >> [1]: >> >> https://lore.kernel.org/all/20240322132619.6389-1-wsa+rene...@sang-engineering.com/ >> >> >> >> Reviewed-by: Rodrigo Vivi >> >> Acked-by: Rodrigo Vivi >> > >> > It looks like the ack is not needed since we are merging this through >> > drm-intel-next. But I'm planing to merge this only after seeing the >> > main drivers/i2c accepting the new terminology. So we don't have a >> > risk of that getting push back and new names there and we having >> > to rename it once again. >> >> Just to be explicit, did you want me to remove the Acked-by in v3, or will >> you when you pull >> the patch into drm-intel-next? >> >> > >> > (more below) >> > >> >> Acked-by: Zhi Wang >> >> Signed-off-by: Easwar Hariharan >> > >> > Cc: Jani Nikula >> > >> > Jani, what bits were you concerned that were not necessarily i2c? >> > I believe although not necessarily/directly i2c, I believe they >> > are related and could benefit from the massive single shot renable. >> > or do you have any better split to suggest here? >> > >> > (more below) >> > >> >> --- >> >> drivers/gpu/drm/i915/display/dvo_ch7017.c | 14 - >> >> drivers/gpu/drm/i915/display/dvo_ch7xxx.c | 18 +-- >> >> drivers/gpu/drm/i915/display/dvo_ivch.c | 16 +- >> >> drivers/gpu/drm/i915/display/dvo_ns2501.c | 18 +-- >> >> drivers/gpu/drm/i915/display/dvo_sil164.c | 18 +-- >> >> drivers/gpu/drm/i915/display/dvo_tfp410.c | 18 +-- >> >> drivers/gpu/drm/i915/display/intel_bios.c | 22 +++--- >> >> drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- >> >> .../gpu/drm/i915/display/intel_display_core.h | 2 +- >> >> drivers/gpu/drm/i915/display/intel_dsi.h | 2 +- >> >> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 20 ++--- >> >> drivers/gpu/drm/i915/display/intel_dvo.c | 14 - >> >> drivers/gpu/drm/i915/display/intel_dvo_dev.h | 2 +- >> >> drivers/gpu/drm/i915/display/intel_gmbus.c| 4 +-- >> >> drivers/gpu/drm/i915/display/intel_sdvo.c | 30 +-- >> >> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 4 +-- >> >> drivers/gpu/drm/i915/gvt/edid.c | 28 - >> >> drivers/gpu/drm/i915/gvt/edid.h | 4 +-- >> >> drivers/gpu/drm/i915/gvt/opregion.c | 2 +- >> >> 19 files changed, 119 insertions(+), 119 deletions(-) >> >> >> >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c >> >> b/drivers/gpu/drm/i915/display/intel_ddi.c >> >> index c17462b4c2ac..64db211148a8 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c >> >> @@ -4332,7 +4332,7 @@ static int intel_ddi_compute_config_late(struct >> >> intel_encoder *encoder, >> >> >> >> connector->tile_group->id); >> >> >> >> /* >> >> - * EDP Transcoders cannot be ensalved >> >> + * EDP Transcoders cannot be slaves >> > >> > ^ here >> > perhaps you meant 'targeted' ? >> > >> >>* make them a master always when present >> >> >> >> This is not actually I2C related as far as I could tell when I was making >> the change, so this was more of a typo fix. >> >> If we want to improve this, a quick check with the eDP v1.5a spec suggests >> using primary/secondary instead, >> though in a global fashion rather than specifically for eDP transcoders. >> There is also source/sink terminology >> in the spec related to DP encoders. >> >> Which would be a more acceptable change here? > > hmmm probably better to split the patches and align with the spec naming > where it applies. > and with i2c name where it applies. Yeah this one is completely unrelated to i2c and aux, and what the eDP spec says is irrelevant here. This should follow Intel hw specs. BR, Jani. -- Jani Nikula, Intel
Re: [PATCH v2 03/12] drm/i915: Make I2C terminology more inclusive
Hi, On Fri, May 03, 2024 at 03:34:12PM -0400, Rodrigo Vivi wrote: > On Fri, May 03, 2024 at 06:13:24PM +, Easwar Hariharan wrote: > > I2C v7, SMBus 3.2, and I3C 1.1.1 specifications have replaced "master/slave" > > with more appropriate terms. Inspired by and following on to Wolfram's > > series to fix drivers/i2c/[1], fix the terminology for users of > > I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists > > in the specification. > > > > Compile tested, no functionality changes intended > > > > [1]: > > https://lore.kernel.org/all/20240322132619.6389-1-wsa+rene...@sang-engineering.com/ > > > > Reviewed-by: Rodrigo Vivi > > Acked-by: Rodrigo Vivi > > It looks like the ack is not needed since we are merging this through > drm-intel-next. But I'm planing to merge this only after seeing the > main drivers/i2c accepting the new terminology. So we don't have a > risk of that getting push back and new names there and we having > to rename it once again. I basically agree with this patch (without the eDP part). But the documentation is still not update and I think we need to hold until Wolfram has done that. In any case, it's good to anticipate the reviews. Thanks Easwar, Andi
Re: [PATCH v2 03/12] drm/i915: Make I2C terminology more inclusive
On 5/3/2024 12:34 PM, Rodrigo Vivi wrote: > On Fri, May 03, 2024 at 06:13:24PM +, Easwar Hariharan wrote: >> I2C v7, SMBus 3.2, and I3C 1.1.1 specifications have replaced "master/slave" >> with more appropriate terms. Inspired by and following on to Wolfram's >> series to fix drivers/i2c/[1], fix the terminology for users of >> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists >> in the specification. >> >> Compile tested, no functionality changes intended >> >> [1]: >> https://lore.kernel.org/all/20240322132619.6389-1-wsa+rene...@sang-engineering.com/ >> >> Reviewed-by: Rodrigo Vivi >> Acked-by: Rodrigo Vivi > > It looks like the ack is not needed since we are merging this through > drm-intel-next. But I'm planing to merge this only after seeing the > main drivers/i2c accepting the new terminology. So we don't have a > risk of that getting push back and new names there and we having > to rename it once again. Just to be explicit, did you want me to remove the Acked-by in v3, or will you when you pull the patch into drm-intel-next? > > (more below) > >> Acked-by: Zhi Wang >> Signed-off-by: Easwar Hariharan > > Cc: Jani Nikula > > Jani, what bits were you concerned that were not necessarily i2c? > I believe although not necessarily/directly i2c, I believe they > are related and could benefit from the massive single shot renable. > or do you have any better split to suggest here? > > (more below) > >> --- >> drivers/gpu/drm/i915/display/dvo_ch7017.c | 14 - >> drivers/gpu/drm/i915/display/dvo_ch7xxx.c | 18 +-- >> drivers/gpu/drm/i915/display/dvo_ivch.c | 16 +- >> drivers/gpu/drm/i915/display/dvo_ns2501.c | 18 +-- >> drivers/gpu/drm/i915/display/dvo_sil164.c | 18 +-- >> drivers/gpu/drm/i915/display/dvo_tfp410.c | 18 +-- >> drivers/gpu/drm/i915/display/intel_bios.c | 22 +++--- >> drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- >> .../gpu/drm/i915/display/intel_display_core.h | 2 +- >> drivers/gpu/drm/i915/display/intel_dsi.h | 2 +- >> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 20 ++--- >> drivers/gpu/drm/i915/display/intel_dvo.c | 14 - >> drivers/gpu/drm/i915/display/intel_dvo_dev.h | 2 +- >> drivers/gpu/drm/i915/display/intel_gmbus.c| 4 +-- >> drivers/gpu/drm/i915/display/intel_sdvo.c | 30 +-- >> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 4 +-- >> drivers/gpu/drm/i915/gvt/edid.c | 28 - >> drivers/gpu/drm/i915/gvt/edid.h | 4 +-- >> drivers/gpu/drm/i915/gvt/opregion.c | 2 +- >> 19 files changed, 119 insertions(+), 119 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c >> b/drivers/gpu/drm/i915/display/intel_ddi.c >> index c17462b4c2ac..64db211148a8 100644 >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c >> @@ -4332,7 +4332,7 @@ static int intel_ddi_compute_config_late(struct >> intel_encoder *encoder, >> >> connector->tile_group->id); >> >> /* >> - * EDP Transcoders cannot be ensalved >> + * EDP Transcoders cannot be slaves > > ^ here > perhaps you meant 'targeted' ? > >> * make them a master always when present This is not actually I2C related as far as I could tell when I was making the change, so this was more of a typo fix. If we want to improve this, a quick check with the eDP v1.5a spec suggests using primary/secondary instead, though in a global fashion rather than specifically for eDP transcoders. There is also source/sink terminology in the spec related to DP encoders. Which would be a more acceptable change here? Thanks, Easwar
