Re: [PATCH v2 1/3] crypto: atmel-sha204a - Drop of_device_id data

["Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>]

[-- Attachment #1: Type: text/plain, Size: 2397 bytes --]

Hello Ard,

On Wed, May 20, 2026 at 09:49:49AM +0200, Ard Biesheuvel wrote:
> On Wed, 20 May 2026, at 09:01, Uwe Kleine-König (The Capable Hub) wrote:
> > The driver binds to i2c devices only and thus in the absence of an
> > assignment for .data in the of_device_id array i2c_get_match_data()
> > falls back to .driver_data from the i2c_device_id array. So only provide
> > &atsha204_quality once to reduce duplication.
> >
> > Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> > ---
> >  drivers/crypto/atmel-sha204a.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> > index 6e6ac4770416..f17e1f6af1a3 100644
> > --- a/drivers/crypto/atmel-sha204a.c
> > +++ b/drivers/crypto/atmel-sha204a.c
> > @@ -208,8 +208,8 @@ static void atmel_sha204a_remove(struct i2c_client *client)
> >  }
> > 
> >  static const struct of_device_id atmel_sha204a_dt_ids[] = {
> > -	{ .compatible = "atmel,atsha204", .data = &atsha204_quality },
> > -	{ .compatible = "atmel,atsha204a", },
> > +	{ .compatible = "atmel,atsha204" },
> > +	{ .compatible = "atmel,atsha204a" },
> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(of, atmel_sha204a_dt_ids);
> 
> Just trying to figure out how this is supposed to work:
> 
> i2c_get_match_data()
>   data = device_get_match_data(&client->dev);
>   ... returns NULL ...
>   if (!data) {
>     match = i2c_match_id(driver->id_table, client);
>     ... compares client->name with { "atsha204", "atsha204a" }
> 
> So we will be relying on client->name having been set to either 
> "atsha204" or "atsha204a" on the DT probe path before
> i2c_match_data() is called, but I am struggling to see where
> that might happen.

That happens when the client is created. Relevant are:

int of_i2c_get_board_info(struct device *dev, struct device_node *node,
	...
{
	...
	if (of_alias_from_compatible(node, info->type, sizeof(info->type)) < 0) {
		...
}

which sets info->type from .compatible with the vendor part skipped.
Then

static struct i2c_client *of_i2c_register_device(...)
{
	...
	ret = of_i2c_get_board_info(&adap->dev, node, &info);
	...
	client = i2c_new_client_device(adap, &info);
	...
}

where i2c_new_client_device() uses info->type to populate client->name.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]