Re: [PATCH v2 1/5] usb: typec: tcpci_rt1711h: Make similar OF and ID table

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Thu, Aug 31, 2023 at 05:04:57PM +0100, Biju Das wrote:
> Make similar OF and ID table to extend support for ID match
> using i2c_match_data() later. Currently it works only for OF match
> tables as the driver_data is wrong for ID match.
> 
> While at it, drop a space from the terminator braces for ID table and
> remove trailing comma in the terminator entry for OF table.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v1->v2:
>  * Drop space from ID table
>  * Remove trailing comma in the terminator entry for OF table.
> ---
>  drivers/usb/typec/tcpm/tcpci_rt1711h.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> index 17ebc5fb684f..5ed3d0864431 100644
> --- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> +++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> @@ -392,9 +392,9 @@ static void rt1711h_remove(struct i2c_client *client)
>  }
>  
>  static const struct i2c_device_id rt1711h_id[] = {
> -	{ "rt1711h", 0 },
> -	{ "rt1715", 0 },
> -	{ }
> +	{ "rt1711h", RT1711H_DID },
> +	{ "rt1715", RT1715_DID },
> +	{}

This is too confusing and messy. Don't assign those RT1711H_DID in
this patch at all - you are not using the I2C driver data anywhere
yet, and in next patch where you start using it, you replace those
assignments with an actual driver data structure that then contain the
values (that did memeber). So there is not point in using the driver
data here at all.

Just make this patch a cleanup patch where you remove the commas, and
deal with those RT1715_DID in the next patch just like you already do.

>  };
>  MODULE_DEVICE_TABLE(i2c, rt1711h_id);
>  
> @@ -402,7 +402,7 @@ MODULE_DEVICE_TABLE(i2c, rt1711h_id);
>  static const struct of_device_id rt1711h_of_match[] = {
>  	{ .compatible = "richtek,rt1711h", .data = (void *)RT1711H_DID },
>  	{ .compatible = "richtek,rt1715", .data = (void *)RT1715_DID },
> -	{},
> +	{}
>  };
>  MODULE_DEVICE_TABLE(of, rt1711h_of_match);
>  #endif

thanks,

-- 
heikki