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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZPWWnOyhoOkLzOlV@kuha.fi.intel.com>
Date:   Mon, 4 Sep 2023 11:34:36 +0300
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Biju Das <biju.das.jz@...renesas.com>
Cc:     Guenter Roeck <linux@...ck-us.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v2 1/5] usb: typec: tcpci_rt1711h: Make similar OF and ID
 table

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@...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