diff mbox series

[v0,13/14] drm/nouveau: Make I2C terminology more inclusive

Message ID 20240329170038.3863998-14-eahariha@linux.microsoft.com (mailing list archive)
State Handled Elsewhere
Headers show
Series Make I2C terminology more inclusive for I2C Algobit and consumers | expand

Commit Message

Easwar Hariharan March 29, 2024, 5 p.m. UTC
I2C v7, SMBus 3.2, and I3C 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+renesas@sang-engineering.com/

Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
---
 drivers/gpu/drm/nouveau/dispnv04/dfp.c             | 14 +++++++-------
 .../gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h |  2 +-
 drivers/gpu/drm/nouveau/nouveau_bios.c             |  4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

Comments

Danilo Krummrich April 5, 2024, 4:15 p.m. UTC | #1
Hi Easwar,

On 3/29/24 18:00, Easwar Hariharan wrote:
> I2C v7, SMBus 3.2, and I3C 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+renesas@sang-engineering.com/
> 
> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
> ---
>   drivers/gpu/drm/nouveau/dispnv04/dfp.c             | 14 +++++++-------
>   .../gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h |  2 +-
>   drivers/gpu/drm/nouveau/nouveau_bios.c             |  4 ++--
>   3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> index d5b129dc623b..65b791006b19 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> @@ -149,7 +149,7 @@ void nv04_dfp_update_fp_control(struct drm_encoder *encoder, int mode)
>   	}
>   }
>   
> -static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
> +static struct drm_encoder *get_tmds_client(struct drm_encoder *encoder)
>   {
>   	struct drm_device *dev = encoder->dev;
>   	struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
> @@ -172,7 +172,7 @@ static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
>   		struct dcb_output *slave_dcb = nouveau_encoder(slave)->dcb;
>   
>   		if (slave_dcb->type == DCB_OUTPUT_TMDS && get_slave_funcs(slave) &&
> -		    slave_dcb->tmdsconf.slave_addr == dcb->tmdsconf.slave_addr)
> +		    slave_dcb->tmdsconf.client_addr == dcb->tmdsconf.client_addr)
>   			return slave;

While, personally, I think master/slave was well suiting for the device relationship
on those busses, I think that if we change it up to conform with the change in
specification, we should make sure to update drivers consistently.

We should make sure to also change the names of the sourrounding structures and variable
names, otherwise we just make this code harder to read.

- Danilo

>   	}
>   
> @@ -471,7 +471,7 @@ static void nv04_dfp_commit(struct drm_encoder *encoder)
>   		NVWriteRAMDAC(dev, 0, NV_PRAMDAC_TEST_CONTROL + nv04_dac_output_offset(encoder), 0x00100000);
>   
>   	/* Init external transmitters */
> -	slave_encoder = get_tmds_slave(encoder);
> +	slave_encoder = get_tmds_client(encoder);
>   	if (slave_encoder)
>   		get_slave_funcs(slave_encoder)->mode_set(
>   			slave_encoder, &nv_encoder->mode, &nv_encoder->mode);
> @@ -621,7 +621,7 @@ static void nv04_dfp_destroy(struct drm_encoder *encoder)
>   	kfree(nv_encoder);
>   }
>   
> -static void nv04_tmds_slave_init(struct drm_encoder *encoder)
> +static void nv04_tmds_client_init(struct drm_encoder *encoder)
>   {
>   	struct drm_device *dev = encoder->dev;
>   	struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
> @@ -632,7 +632,7 @@ static void nv04_tmds_slave_init(struct drm_encoder *encoder)
>   		{
>   		    {
>   		        .type = "sil164",
> -		        .addr = (dcb->tmdsconf.slave_addr == 0x7 ? 0x3a : 0x38),
> +		        .addr = (dcb->tmdsconf.client_addr == 0x7 ? 0x3a : 0x38),
>   		        .platform_data = &(struct sil164_encoder_params) {
>   		            SIL164_INPUT_EDGE_RISING
>   		         }
> @@ -642,7 +642,7 @@ static void nv04_tmds_slave_init(struct drm_encoder *encoder)
>   	};
>   	int type;
>   
> -	if (!nv_gf4_disp_arch(dev) || !bus || get_tmds_slave(encoder))
> +	if (!nv_gf4_disp_arch(dev) || !bus || get_tmds_client(encoder))
>   		return;
>   
>   	type = nvkm_i2c_bus_probe(bus, "TMDS transmitter", info, NULL, NULL);
> @@ -716,7 +716,7 @@ nv04_dfp_create(struct drm_connector *connector, struct dcb_output *entry)
>   
>   	if (entry->type == DCB_OUTPUT_TMDS &&
>   	    entry->location != DCB_LOC_ON_CHIP)
> -		nv04_tmds_slave_init(encoder);
> +		nv04_tmds_client_init(encoder);
>   
>   	drm_connector_attach_encoder(connector, encoder);
>   	return 0;
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h
> index 73f9d9947e7e..5da40cf74b1a 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h
> @@ -50,7 +50,7 @@ struct dcb_output {
>   		} dpconf;
>   		struct {
>   			struct sor_conf sor;
> -			int slave_addr;
> +			int client_addr;
>   		} tmdsconf;
>   	};
>   	bool i2c_upper_default;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c b/drivers/gpu/drm/nouveau/nouveau_bios.c
> index 479effcf607e..a91a5d3df3ca 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bios.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
> @@ -1511,9 +1511,9 @@ parse_dcb20_entry(struct drm_device *dev, struct dcb_table *dcb,
>   			link = entry->tmdsconf.sor.link;
>   		}
>   		else if (dcb->version >= 0x30)
> -			entry->tmdsconf.slave_addr = (conf & 0x00000700) >> 8;
> +			entry->tmdsconf.client_addr = (conf & 0x00000700) >> 8;
>   		else if (dcb->version >= 0x22)
> -			entry->tmdsconf.slave_addr = (conf & 0x00000070) >> 4;
> +			entry->tmdsconf.client_addr = (conf & 0x00000070) >> 4;
>   		break;
>   	case DCB_OUTPUT_EOL:
>   		/* weird g80 mobile type that "nv" treats as a terminator */
Easwar Hariharan April 5, 2024, 4:30 p.m. UTC | #2
On 4/5/2024 9:15 AM, Danilo Krummrich wrote:
> Hi Easwar,
> 
> On 3/29/24 18:00, Easwar Hariharan wrote:
>> I2C v7, SMBus 3.2, and I3C 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+renesas@sang-engineering.com/
>>
>> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
>> ---
>>   drivers/gpu/drm/nouveau/dispnv04/dfp.c             | 14 +++++++-------
>>   .../gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h |  2 +-
>>   drivers/gpu/drm/nouveau/nouveau_bios.c             |  4 ++--
>>   3 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> index d5b129dc623b..65b791006b19 100644
>> --- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> +++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> @@ -149,7 +149,7 @@ void nv04_dfp_update_fp_control(struct drm_encoder *encoder, int mode)
>>       }
>>   }
>>   -static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
>> +static struct drm_encoder *get_tmds_client(struct drm_encoder *encoder)
>>   {
>>       struct drm_device *dev = encoder->dev;
>>       struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
>> @@ -172,7 +172,7 @@ static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
>>           struct dcb_output *slave_dcb = nouveau_encoder(slave)->dcb;
>>             if (slave_dcb->type == DCB_OUTPUT_TMDS && get_slave_funcs(slave) &&
>> -            slave_dcb->tmdsconf.slave_addr == dcb->tmdsconf.slave_addr)
>> +            slave_dcb->tmdsconf.client_addr == dcb->tmdsconf.client_addr)
>>               return slave;
> 
> While, personally, I think master/slave was well suiting for the device relationship
> on those busses, I think that if we change it up to conform with the change in
> specification, we should make sure to update drivers consistently.
> 
> We should make sure to also change the names of the sourrounding structures and variable
> names, otherwise we just make this code harder to read.
> 
> - Danilo
> 

Thanks for the review, and for the appetite to go further! So we are on the same page, you would prefer
renaming to controller/target like the feedback on other drm drivers (i915, gma500, radeon)?

Thanks,
Easwar
Lyude Paul April 5, 2024, 6:36 p.m. UTC | #3
On Fri, 2024-04-05 at 09:30 -0700, Easwar Hariharan wrote:
> 
> Thanks for the review, and for the appetite to go further! So we are
> on the same page, you would prefer
> renaming to controller/target like the feedback on other drm drivers
> (i915, gma500, radeon)?

FWIW I'm in support of this as well! As long as we make sure it gets
renamed everywhere :)

> 
> Thanks,
> Easwar
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
index d5b129dc623b..65b791006b19 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
@@ -149,7 +149,7 @@  void nv04_dfp_update_fp_control(struct drm_encoder *encoder, int mode)
 	}
 }
 
-static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
+static struct drm_encoder *get_tmds_client(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
 	struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
@@ -172,7 +172,7 @@  static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
 		struct dcb_output *slave_dcb = nouveau_encoder(slave)->dcb;
 
 		if (slave_dcb->type == DCB_OUTPUT_TMDS && get_slave_funcs(slave) &&
-		    slave_dcb->tmdsconf.slave_addr == dcb->tmdsconf.slave_addr)
+		    slave_dcb->tmdsconf.client_addr == dcb->tmdsconf.client_addr)
 			return slave;
 	}
 
@@ -471,7 +471,7 @@  static void nv04_dfp_commit(struct drm_encoder *encoder)
 		NVWriteRAMDAC(dev, 0, NV_PRAMDAC_TEST_CONTROL + nv04_dac_output_offset(encoder), 0x00100000);
 
 	/* Init external transmitters */
-	slave_encoder = get_tmds_slave(encoder);
+	slave_encoder = get_tmds_client(encoder);
 	if (slave_encoder)
 		get_slave_funcs(slave_encoder)->mode_set(
 			slave_encoder, &nv_encoder->mode, &nv_encoder->mode);
@@ -621,7 +621,7 @@  static void nv04_dfp_destroy(struct drm_encoder *encoder)
 	kfree(nv_encoder);
 }
 
-static void nv04_tmds_slave_init(struct drm_encoder *encoder)
+static void nv04_tmds_client_init(struct drm_encoder *encoder)
 {
 	struct drm_device *dev = encoder->dev;
 	struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
@@ -632,7 +632,7 @@  static void nv04_tmds_slave_init(struct drm_encoder *encoder)
 		{
 		    {
 		        .type = "sil164",
-		        .addr = (dcb->tmdsconf.slave_addr == 0x7 ? 0x3a : 0x38),
+		        .addr = (dcb->tmdsconf.client_addr == 0x7 ? 0x3a : 0x38),
 		        .platform_data = &(struct sil164_encoder_params) {
 		            SIL164_INPUT_EDGE_RISING
 		         }
@@ -642,7 +642,7 @@  static void nv04_tmds_slave_init(struct drm_encoder *encoder)
 	};
 	int type;
 
-	if (!nv_gf4_disp_arch(dev) || !bus || get_tmds_slave(encoder))
+	if (!nv_gf4_disp_arch(dev) || !bus || get_tmds_client(encoder))
 		return;
 
 	type = nvkm_i2c_bus_probe(bus, "TMDS transmitter", info, NULL, NULL);
@@ -716,7 +716,7 @@  nv04_dfp_create(struct drm_connector *connector, struct dcb_output *entry)
 
 	if (entry->type == DCB_OUTPUT_TMDS &&
 	    entry->location != DCB_LOC_ON_CHIP)
-		nv04_tmds_slave_init(encoder);
+		nv04_tmds_client_init(encoder);
 
 	drm_connector_attach_encoder(connector, encoder);
 	return 0;
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h
index 73f9d9947e7e..5da40cf74b1a 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h
@@ -50,7 +50,7 @@  struct dcb_output {
 		} dpconf;
 		struct {
 			struct sor_conf sor;
-			int slave_addr;
+			int client_addr;
 		} tmdsconf;
 	};
 	bool i2c_upper_default;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c b/drivers/gpu/drm/nouveau/nouveau_bios.c
index 479effcf607e..a91a5d3df3ca 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bios.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
@@ -1511,9 +1511,9 @@  parse_dcb20_entry(struct drm_device *dev, struct dcb_table *dcb,
 			link = entry->tmdsconf.sor.link;
 		}
 		else if (dcb->version >= 0x30)
-			entry->tmdsconf.slave_addr = (conf & 0x00000700) >> 8;
+			entry->tmdsconf.client_addr = (conf & 0x00000700) >> 8;
 		else if (dcb->version >= 0x22)
-			entry->tmdsconf.slave_addr = (conf & 0x00000070) >> 4;
+			entry->tmdsconf.client_addr = (conf & 0x00000070) >> 4;
 		break;
 	case DCB_OUTPUT_EOL:
 		/* weird g80 mobile type that "nv" treats as a terminator */