| Message ID | 20230828180003.127896-1-biju.das.jz@bp.renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] ASoC: ak4642: Simplify probe() | expand |
On Mon, Aug 28, 2023 at 07:00:03PM +0100, Biju Das wrote: > Simpilfy probe() by replacing of_device_get_match_data() and id lookup for > retrieving match data by i2c_get_match_data() and replace > dev_err()->dev_err_probe(). ... > - if (np) { > - const struct of_device_id *of_id; > - > + if (dev_fwnode(dev)) { Why do we need this at all? > mcko = ak4642_of_parse_mcko(dev); > if (IS_ERR(mcko)) > mcko = NULL; This can suffice on its own, right? Can be done in a separate change as a precursor to this one. > - > - of_id = of_match_device(ak4642_of_match, dev); > - if (of_id) > - drvdata = of_id->data; > - } else { > - const struct i2c_device_id *id = > - i2c_match_id(ak4642_i2c_id, i2c); > - drvdata = (const struct ak4642_drvdata *)id->driver_data; > }
Hi Andy, > Subject: Re: [PATCH v2] ASoC: ak4642: Simplify probe() > > On Mon, Aug 28, 2023 at 07:00:03PM +0100, Biju Das wrote: > > Simpilfy probe() by replacing of_device_get_match_data() and id lookup > > for retrieving match data by i2c_get_match_data() and replace > > dev_err()->dev_err_probe(). > > ... > > > - if (np) { > > - const struct of_device_id *of_id; > > - > > > + if (dev_fwnode(dev)) { > > Why do we need this at all? It is replacement for np. > > > mcko = ak4642_of_parse_mcko(dev); > > if (IS_ERR(mcko)) > > mcko = NULL; > > This can suffice on its own, right? > > Can be done in a separate change as a precursor to this one. Agreed. Cheers, Biju > > > - > > - of_id = of_match_device(ak4642_of_match, dev); > > - if (of_id) > > - drvdata = of_id->data; > > - } else { > > - const struct i2c_device_id *id = > > - i2c_match_id(ak4642_i2c_id, i2c); > > - drvdata = (const struct ak4642_drvdata *)id->driver_data; > > } > > -- > With Best Regards, > Andy Shevchenko >
On Tue, Aug 29, 2023 at 06:02:51PM +0000, Biju Das wrote: > > On Mon, Aug 28, 2023 at 07:00:03PM +0100, Biju Das wrote: ... > > > - if (np) { > > > - const struct of_device_id *of_id; > > > - > > > > > + if (dev_fwnode(dev)) { > > > > Why do we need this at all? > It is replacement for np. I am questioning it's necessity to begin with (even before your patch). > > > mcko = ak4642_of_parse_mcko(dev); > > > if (IS_ERR(mcko)) > > > mcko = NULL; > > > > This can suffice on its own, right? > > > > Can be done in a separate change as a precursor to this one. > > Agreed.
Hi Andy, > Subject: Re: [PATCH v2] ASoC: ak4642: Simplify probe() > > On Tue, Aug 29, 2023 at 06:02:51PM +0000, Biju Das wrote: > > > On Mon, Aug 28, 2023 at 07:00:03PM +0100, Biju Das wrote: > > ... > > > > > - if (np) { > > > > - const struct of_device_id *of_id; > > > > - > > > > > > > + if (dev_fwnode(dev)) { > > > > > > Why do we need this at all? > > It is replacement for np. > > I am questioning it's necessity to begin with (even before your patch). OK, I will make separate patch as precursor to this one if (dev_fwnode(dev)) { mcko = ak4642_of_parse_mcko(dev); if (IS_ERR(mcko)) mcko = NULL; } Cheers, Biju > > > > > mcko = ak4642_of_parse_mcko(dev); > > > > if (IS_ERR(mcko)) > > > > mcko = NULL; > > > > > > This can suffice on its own, right? > > > > > > Can be done in a separate change as a precursor to this one. > > > > Agreed. > > -- > With Best Regards, > Andy Shevchenko >
diff --git a/sound/soc/codecs/ak4642.c b/sound/soc/codecs/ak4642.c index 2a8984c1fa9c..8a40c6b3f4d8 100644 --- a/sound/soc/codecs/ak4642.c +++ b/sound/soc/codecs/ak4642.c @@ -628,37 +628,23 @@ static struct clk *ak4642_of_parse_mcko(struct device *dev) #define ak4642_of_parse_mcko(d) 0 #endif -static const struct of_device_id ak4642_of_match[]; -static const struct i2c_device_id ak4642_i2c_id[]; static int ak4642_i2c_probe(struct i2c_client *i2c) { struct device *dev = &i2c->dev; - struct device_node *np = dev->of_node; - const struct ak4642_drvdata *drvdata = NULL; + const struct ak4642_drvdata *drvdata; struct regmap *regmap; struct ak4642_priv *priv; struct clk *mcko = NULL; - if (np) { - const struct of_device_id *of_id; - + if (dev_fwnode(dev)) { mcko = ak4642_of_parse_mcko(dev); if (IS_ERR(mcko)) mcko = NULL; - - of_id = of_match_device(ak4642_of_match, dev); - if (of_id) - drvdata = of_id->data; - } else { - const struct i2c_device_id *id = - i2c_match_id(ak4642_i2c_id, i2c); - drvdata = (const struct ak4642_drvdata *)id->driver_data; } - if (!drvdata) { - dev_err(dev, "Unknown device type\n"); - return -EINVAL; - } + drvdata = i2c_get_match_data(i2c); + if (!drvdata) + return dev_err_probe(dev, -EINVAL, "Unknown device type\n"); priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) @@ -681,7 +667,7 @@ static const struct of_device_id ak4642_of_match[] = { { .compatible = "asahi-kasei,ak4642", .data = &ak4642_drvdata}, { .compatible = "asahi-kasei,ak4643", .data = &ak4643_drvdata}, { .compatible = "asahi-kasei,ak4648", .data = &ak4648_drvdata}, - {}, + {} }; MODULE_DEVICE_TABLE(of, ak4642_of_match); @@ -689,7 +675,7 @@ static const struct i2c_device_id ak4642_i2c_id[] = { { "ak4642", (kernel_ulong_t)&ak4642_drvdata }, { "ak4643", (kernel_ulong_t)&ak4643_drvdata }, { "ak4648", (kernel_ulong_t)&ak4648_drvdata }, - { } + {} }; MODULE_DEVICE_TABLE(i2c, ak4642_i2c_id);
Simpilfy probe() by replacing of_device_get_match_data() and id lookup for retrieving match data by i2c_get_match_data() and replace dev_err()->dev_err_probe(). While at it, drop local variable np and use dev_fwnode() instead and remove comma in the terminator entry. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- Note: This patch is only compile tested. v1->v2: * Removed forward declaration ak4642_of_match and ak4642_i2c_id. * Restored error EINVAL. * Used dev_fwnode() and replaced dev->of_node. * Removed comma in the terminator entry. --- sound/soc/codecs/ak4642.c | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-)