| Message ID | 20230827094536.49511-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | ASoC: tlv320aic32x4-i2c: Simplify probe() | expand |
On Sun, Aug 27, 2023 at 10:45:36AM +0100, Biju Das wrote: > Simplify probe() by replacing of_match_node() and i2c_match_id() with > i2c_get_match_data(). ... > + dev_set_drvdata(&i2c->dev, (void *)i2c_get_match_data(i2c)); You (potentially) drop const qualifier here. It's not good and it's not explained in the commit message why.
Hi Andy Shevchenko, > Subject: Re: [PATCH] ASoC: tlv320aic32x4-i2c: Simplify probe() > > On Sun, Aug 27, 2023 at 10:45:36AM +0100, Biju Das wrote: > > Simplify probe() by replacing of_match_node() and i2c_match_id() with > > i2c_get_match_data(). > > ... > > > + dev_set_drvdata(&i2c->dev, (void *)i2c_get_match_data(i2c)); > > You (potentially) drop const qualifier here. It's not good and it's not > explained in the commit message why. dev_set_drvdata() needs non-const void*, otherwise I get warning. The original code also use this cast. That is the reason it is not explained in commit message. Cheers, Biju
On Mon, Aug 28, 2023 at 11:48:28AM +0000, Biju Das wrote: > > On Sun, Aug 27, 2023 at 10:45:36AM +0100, Biju Das wrote: ... > > > + dev_set_drvdata(&i2c->dev, (void *)i2c_get_match_data(i2c)); > > > > You (potentially) drop const qualifier here. It's not good and it's not > > explained in the commit message why. > > dev_set_drvdata() needs non-const void*, otherwise I get warning. > The original code also use this cast. That is the reason it is not > explained in commit message. Maybe it shouldn't use this pointer directly then? aic32x4_probe() has these lines aic32x4->type = (uintptr_t)dev_get_drvdata(dev); dev_set_drvdata(dev, aic32x4); Dragging data like this makes a little sense to me. What you should do is to have a precursor patch that adds a third parameter to ->probe(). Also it makes sense to all your improvements for -i2c.c do similar to all -spi.c cases where it applies.
diff --git a/sound/soc/codecs/tlv320aic32x4-i2c.c b/sound/soc/codecs/tlv320aic32x4-i2c.c index 49b33a256cd7..513f257818ca 100644 --- a/sound/soc/codecs/tlv320aic32x4-i2c.c +++ b/sound/soc/codecs/tlv320aic32x4-i2c.c @@ -16,9 +16,6 @@ #include "tlv320aic32x4.h" -static const struct of_device_id aic32x4_of_id[]; -static const struct i2c_device_id aic32x4_i2c_id[]; - static int aic32x4_i2c_probe(struct i2c_client *i2c) { struct regmap *regmap; @@ -30,17 +27,7 @@ static int aic32x4_i2c_probe(struct i2c_client *i2c) regmap = devm_regmap_init_i2c(i2c, &config); - if (i2c->dev.of_node) { - const struct of_device_id *oid; - - oid = of_match_node(aic32x4_of_id, i2c->dev.of_node); - dev_set_drvdata(&i2c->dev, (void *)oid->data); - } else { - const struct i2c_device_id *id; - - id = i2c_match_id(aic32x4_i2c_id, i2c); - dev_set_drvdata(&i2c->dev, (void *)id->driver_data); - } + dev_set_drvdata(&i2c->dev, (void *)i2c_get_match_data(i2c)); return aic32x4_probe(&i2c->dev, regmap); }
Simplify probe() by replacing of_match_node() and i2c_match_id() with i2c_get_match_data(). Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- Note: This patch is only compile tested. --- sound/soc/codecs/tlv320aic32x4-i2c.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-)