* [PATCH v2 0/2] Use i2c_get_match_data() for mcp4725 DAC
@ 2023-08-18 17:49 Biju Das
2023-08-18 17:49 ` [PATCH v2 1/2] iio: dac: mcp4725: Replace variable 'id' from struct mcp4725_data Biju Das
2023-08-18 17:49 ` [PATCH v2 2/2] iio: dac: mcp4725: Use i2c_get_match_data() Biju Das
0 siblings, 2 replies; 5+ messages in thread
From: Biju Das @ 2023-08-18 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Biju Das, Lars-Peter Clausen, Uwe Kleine-König,
Andy Shevchenko, Marek Vasut, linux-iio, Geert Uytterhoeven,
Prabhakar Mahadev Lad, linux-renesas-soc
Use i2c_get_match_data() to get match data for I2C, ACPI and DT-based
matching instead of device_get_match_data() the one for ACPI/DT-based
matching.
This patch series is only compile tested.
v1->v2:
* Updated commit description for patch#1 with reason for change as
mentioned by Marek.
Biju Das (2):
iio: dac: mcp4725: Replace variable 'id' from struct mcp4725_data
iio: dac: mcp4725: Use i2c_get_match_data()
drivers/iio/dac/mcp4725.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] iio: dac: mcp4725: Replace variable 'id' from struct mcp4725_data
2023-08-18 17:49 [PATCH v2 0/2] Use i2c_get_match_data() for mcp4725 DAC Biju Das
@ 2023-08-18 17:49 ` Biju Das
2023-08-18 17:49 ` [PATCH v2 2/2] iio: dac: mcp4725: Use i2c_get_match_data() Biju Das
1 sibling, 0 replies; 5+ messages in thread
From: Biju Das @ 2023-08-18 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Biju Das, Lars-Peter Clausen, Uwe Kleine-König,
Andy Shevchenko, Marek Vasut, linux-iio, Geert Uytterhoeven,
Prabhakar Mahadev Lad, linux-renesas-soc
Replace the variable 'id' from struct mcp4725_data with local variable
chip_id in probe() as the id variable is not used elsewhere in the
driver.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
* Update commit description with reason for change.
---
drivers/iio/dac/mcp4725.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
index f4a3124d29f2..33a61f65bc25 100644
--- a/drivers/iio/dac/mcp4725.c
+++ b/drivers/iio/dac/mcp4725.c
@@ -32,7 +32,6 @@
struct mcp4725_data {
struct i2c_client *client;
- int id;
unsigned ref_mode;
bool vref_buffered;
u16 dac_value;
@@ -387,6 +386,7 @@ static int mcp4725_probe(struct i2c_client *client)
struct mcp4725_data *data;
struct iio_dev *indio_dev;
struct mcp4725_platform_data *pdata, pdata_dt;
+ int chip_id;
u8 inbuf[4];
u8 pd;
u8 ref;
@@ -399,9 +399,9 @@ static int mcp4725_probe(struct i2c_client *client)
i2c_set_clientdata(client, indio_dev);
data->client = client;
if (dev_fwnode(&client->dev))
- data->id = (uintptr_t)device_get_match_data(&client->dev);
+ chip_id = (uintptr_t)device_get_match_data(&client->dev);
else
- data->id = id->driver_data;
+ chip_id = id->driver_data;
pdata = dev_get_platdata(&client->dev);
if (!pdata) {
@@ -414,7 +414,7 @@ static int mcp4725_probe(struct i2c_client *client)
pdata = &pdata_dt;
}
- if (data->id == MCP4725 && pdata->use_vref) {
+ if (chip_id == MCP4725 && pdata->use_vref) {
dev_err(&client->dev,
"external reference is unavailable on MCP4725");
return -EINVAL;
@@ -455,12 +455,12 @@ static int mcp4725_probe(struct i2c_client *client)
indio_dev->name = id->name;
indio_dev->info = &mcp4725_info;
- indio_dev->channels = &mcp472x_channel[id->driver_data];
+ indio_dev->channels = &mcp472x_channel[chip_id];
indio_dev->num_channels = 1;
indio_dev->modes = INDIO_DIRECT_MODE;
/* read current DAC value and settings */
- err = i2c_master_recv(client, inbuf, data->id == MCP4725 ? 3 : 4);
+ err = i2c_master_recv(client, inbuf, chip_id == MCP4725 ? 3 : 4);
if (err < 0) {
dev_err(&client->dev, "failed to read DAC value");
@@ -470,10 +470,10 @@ static int mcp4725_probe(struct i2c_client *client)
data->powerdown = pd > 0;
data->powerdown_mode = pd ? pd - 1 : 2; /* largest resistor to gnd */
data->dac_value = (inbuf[1] << 4) | (inbuf[2] >> 4);
- if (data->id == MCP4726)
+ if (chip_id == MCP4726)
ref = (inbuf[3] >> 3) & 0x3;
- if (data->id == MCP4726 && ref != data->ref_mode) {
+ if (chip_id == MCP4726 && ref != data->ref_mode) {
dev_info(&client->dev,
"voltage reference mode differs (conf: %u, eeprom: %u), setting %u",
data->ref_mode, ref, data->ref_mode);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] iio: dac: mcp4725: Use i2c_get_match_data()
2023-08-18 17:49 [PATCH v2 0/2] Use i2c_get_match_data() for mcp4725 DAC Biju Das
2023-08-18 17:49 ` [PATCH v2 1/2] iio: dac: mcp4725: Replace variable 'id' from struct mcp4725_data Biju Das
@ 2023-08-18 17:49 ` Biju Das
2023-08-28 12:45 ` Jonathan Cameron
1 sibling, 1 reply; 5+ messages in thread
From: Biju Das @ 2023-08-18 17:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Biju Das, Lars-Peter Clausen, Uwe Kleine-König,
Andy Shevchenko, Marek Vasut, linux-iio, Geert Uytterhoeven,
Prabhakar Mahadev Lad, linux-renesas-soc
Replace local variable chip_id with struct iio_chan_spec for device data
and use its .ext_info for chip identification. After this replace
device_get_match_data() and id lookup for retrieving match data
by i2c_get_match_data() by converting enum->pointer for data in the
match table.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
* No change.
---
drivers/iio/dac/mcp4725.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
index 33a61f65bc25..f9ce01c4cc53 100644
--- a/drivers/iio/dac/mcp4725.c
+++ b/drivers/iio/dac/mcp4725.c
@@ -386,7 +386,7 @@ static int mcp4725_probe(struct i2c_client *client)
struct mcp4725_data *data;
struct iio_dev *indio_dev;
struct mcp4725_platform_data *pdata, pdata_dt;
- int chip_id;
+ const struct iio_chan_spec *ch;
u8 inbuf[4];
u8 pd;
u8 ref;
@@ -398,10 +398,8 @@ static int mcp4725_probe(struct i2c_client *client)
data = iio_priv(indio_dev);
i2c_set_clientdata(client, indio_dev);
data->client = client;
- if (dev_fwnode(&client->dev))
- chip_id = (uintptr_t)device_get_match_data(&client->dev);
- else
- chip_id = id->driver_data;
+ ch = i2c_get_match_data(client);
+
pdata = dev_get_platdata(&client->dev);
if (!pdata) {
@@ -414,7 +412,7 @@ static int mcp4725_probe(struct i2c_client *client)
pdata = &pdata_dt;
}
- if (chip_id == MCP4725 && pdata->use_vref) {
+ if (ch->ext_info == mcp4725_ext_info && pdata->use_vref) {
dev_err(&client->dev,
"external reference is unavailable on MCP4725");
return -EINVAL;
@@ -455,12 +453,12 @@ static int mcp4725_probe(struct i2c_client *client)
indio_dev->name = id->name;
indio_dev->info = &mcp4725_info;
- indio_dev->channels = &mcp472x_channel[chip_id];
+ indio_dev->channels = ch;
indio_dev->num_channels = 1;
indio_dev->modes = INDIO_DIRECT_MODE;
/* read current DAC value and settings */
- err = i2c_master_recv(client, inbuf, chip_id == MCP4725 ? 3 : 4);
+ err = i2c_master_recv(client, inbuf, ch->ext_info == mcp4725_ext_info ? 3 : 4);
if (err < 0) {
dev_err(&client->dev, "failed to read DAC value");
@@ -470,10 +468,10 @@ static int mcp4725_probe(struct i2c_client *client)
data->powerdown = pd > 0;
data->powerdown_mode = pd ? pd - 1 : 2; /* largest resistor to gnd */
data->dac_value = (inbuf[1] << 4) | (inbuf[2] >> 4);
- if (chip_id == MCP4726)
+ if (ch->ext_info == mcp4726_ext_info)
ref = (inbuf[3] >> 3) & 0x3;
- if (chip_id == MCP4726 && ref != data->ref_mode) {
+ if (ch->ext_info == mcp4726_ext_info && ref != data->ref_mode) {
dev_info(&client->dev,
"voltage reference mode differs (conf: %u, eeprom: %u), setting %u",
data->ref_mode, ref, data->ref_mode);
@@ -511,8 +509,8 @@ static void mcp4725_remove(struct i2c_client *client)
}
static const struct i2c_device_id mcp4725_id[] = {
- { "mcp4725", MCP4725 },
- { "mcp4726", MCP4726 },
+ { "mcp4725", (kernel_ulong_t)&mcp472x_channel[MCP4725] },
+ { "mcp4726", (kernel_ulong_t)&mcp472x_channel[MCP4726] },
{ }
};
MODULE_DEVICE_TABLE(i2c, mcp4725_id);
@@ -520,11 +518,11 @@ MODULE_DEVICE_TABLE(i2c, mcp4725_id);
static const struct of_device_id mcp4725_of_match[] = {
{
.compatible = "microchip,mcp4725",
- .data = (void *)MCP4725
+ .data = &mcp472x_channel[MCP4725]
},
{
.compatible = "microchip,mcp4726",
- .data = (void *)MCP4726
+ .data = &mcp472x_channel[MCP4726]
},
{ }
};
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] iio: dac: mcp4725: Use i2c_get_match_data()
2023-08-18 17:49 ` [PATCH v2 2/2] iio: dac: mcp4725: Use i2c_get_match_data() Biju Das
@ 2023-08-28 12:45 ` Jonathan Cameron
2023-08-29 14:35 ` Biju Das
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2023-08-28 12:45 UTC (permalink / raw)
To: Biju Das
Cc: Lars-Peter Clausen, Uwe Kleine-König, Andy Shevchenko,
Marek Vasut, linux-iio, Geert Uytterhoeven,
Prabhakar Mahadev Lad, linux-renesas-soc
On Fri, 18 Aug 2023 18:49:05 +0100
Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Replace local variable chip_id with struct iio_chan_spec for device data
> and use its .ext_info for chip identification. After this replace
> device_get_match_data() and id lookup for retrieving match data
> by i2c_get_match_data() by converting enum->pointer for data in the
> match table.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
The use of ext_info on a channel is messy. Please just define
a clearly named flag in chip_info to make it simpler to maintain
and extend.
Jonathan
> ---
> v1->v2:
> * No change.
> ---
> drivers/iio/dac/mcp4725.c | 26 ++++++++++++--------------
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
> index 33a61f65bc25..f9ce01c4cc53 100644
> --- a/drivers/iio/dac/mcp4725.c
> +++ b/drivers/iio/dac/mcp4725.c
> @@ -386,7 +386,7 @@ static int mcp4725_probe(struct i2c_client *client)
> struct mcp4725_data *data;
> struct iio_dev *indio_dev;
> struct mcp4725_platform_data *pdata, pdata_dt;
> - int chip_id;
> + const struct iio_chan_spec *ch;
> u8 inbuf[4];
> u8 pd;
> u8 ref;
> @@ -398,10 +398,8 @@ static int mcp4725_probe(struct i2c_client *client)
> data = iio_priv(indio_dev);
> i2c_set_clientdata(client, indio_dev);
> data->client = client;
> - if (dev_fwnode(&client->dev))
> - chip_id = (uintptr_t)device_get_match_data(&client->dev);
> - else
> - chip_id = id->driver_data;
> + ch = i2c_get_match_data(client);
> +
> pdata = dev_get_platdata(&client->dev);
>
> if (!pdata) {
> @@ -414,7 +412,7 @@ static int mcp4725_probe(struct i2c_client *client)
> pdata = &pdata_dt;
> }
>
> - if (chip_id == MCP4725 && pdata->use_vref) {
> + if (ch->ext_info == mcp4725_ext_info && pdata->use_vref) {
I think this will end up being hard to maintain.
Please define a mcp4725_chip_info structure and put the channels in there
+ a flag that says if the device in question supports an external reference.
> dev_err(&client->dev,
> "external reference is unavailable on MCP4725");
> return -EINVAL;
> @@ -455,12 +453,12 @@ static int mcp4725_probe(struct i2c_client *client)
>
> indio_dev->name = id->name;
> indio_dev->info = &mcp4725_info;
> - indio_dev->channels = &mcp472x_channel[chip_id];
> + indio_dev->channels = ch;
> indio_dev->num_channels = 1;
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> /* read current DAC value and settings */
> - err = i2c_master_recv(client, inbuf, chip_id == MCP4725 ? 3 : 4);
> + err = i2c_master_recv(client, inbuf, ch->ext_info == mcp4725_ext_info ? 3 : 4);
>
> if (err < 0) {
> dev_err(&client->dev, "failed to read DAC value");
> @@ -470,10 +468,10 @@ static int mcp4725_probe(struct i2c_client *client)
> data->powerdown = pd > 0;
> data->powerdown_mode = pd ? pd - 1 : 2; /* largest resistor to gnd */
> data->dac_value = (inbuf[1] << 4) | (inbuf[2] >> 4);
> - if (chip_id == MCP4726)
> + if (ch->ext_info == mcp4726_ext_info)
> ref = (inbuf[3] >> 3) & 0x3;
Add a flag to the chip_info structure for whatever we are controlling here
and then have
if (info->x_is_supported)
...
>
> - if (chip_id == MCP4726 && ref != data->ref_mode) {
> + if (ch->ext_info == mcp4726_ext_info && ref != data->ref_mode) {
I think this can also use the vref bool I suggest adding above.
> dev_info(&client->dev,
> "voltage reference mode differs (conf: %u, eeprom: %u), setting %u",
> data->ref_mode, ref, data->ref_mode);
> @@ -511,8 +509,8 @@ static void mcp4725_remove(struct i2c_client *client)
> }
>
> static const struct i2c_device_id mcp4725_id[] = {
> - { "mcp4725", MCP4725 },
> - { "mcp4726", MCP4726 },
> + { "mcp4725", (kernel_ulong_t)&mcp472x_channel[MCP4725] },
> + { "mcp4726", (kernel_ulong_t)&mcp472x_channel[MCP4726] },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, mcp4725_id);
> @@ -520,11 +518,11 @@ MODULE_DEVICE_TABLE(i2c, mcp4725_id);
> static const struct of_device_id mcp4725_of_match[] = {
> {
> .compatible = "microchip,mcp4725",
> - .data = (void *)MCP4725
> + .data = &mcp472x_channel[MCP4725]
> },
> {
> .compatible = "microchip,mcp4726",
> - .data = (void *)MCP4726
> + .data = &mcp472x_channel[MCP4726]
> },
> { }
> };
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2 2/2] iio: dac: mcp4725: Use i2c_get_match_data()
2023-08-28 12:45 ` Jonathan Cameron
@ 2023-08-29 14:35 ` Biju Das
0 siblings, 0 replies; 5+ messages in thread
From: Biju Das @ 2023-08-29 14:35 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Uwe Kleine-König, Andy Shevchenko,
Marek Vasut, linux-iio
Hi Jonathan Cameron,
> Subject: Re: [PATCH v2 2/2] iio: dac: mcp4725: Use i2c_get_match_data()
>
> On Fri, 18 Aug 2023 18:49:05 +0100
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> > Replace local variable chip_id with struct iio_chan_spec for device
> > data and use its .ext_info for chip identification. After this replace
> > device_get_match_data() and id lookup for retrieving match data by
> > i2c_get_match_data() by converting enum->pointer for data in the match
> > table.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> The use of ext_info on a channel is messy. Please just define a clearly
> named flag in chip_info to make it simpler to maintain and extend.
OK
> Jonathan
>
> > ---
> > v1->v2:
> > * No change.
> > ---
> > drivers/iio/dac/mcp4725.c | 26 ++++++++++++--------------
> > 1 file changed, 12 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
> > index 33a61f65bc25..f9ce01c4cc53 100644
> > --- a/drivers/iio/dac/mcp4725.c
> > +++ b/drivers/iio/dac/mcp4725.c
> > @@ -386,7 +386,7 @@ static int mcp4725_probe(struct i2c_client *client)
> > struct mcp4725_data *data;
> > struct iio_dev *indio_dev;
> > struct mcp4725_platform_data *pdata, pdata_dt;
> > - int chip_id;
> > + const struct iio_chan_spec *ch;
> > u8 inbuf[4];
> > u8 pd;
> > u8 ref;
> > @@ -398,10 +398,8 @@ static int mcp4725_probe(struct i2c_client *client)
> > data = iio_priv(indio_dev);
> > i2c_set_clientdata(client, indio_dev);
> > data->client = client;
> > - if (dev_fwnode(&client->dev))
> > - chip_id = (uintptr_t)device_get_match_data(&client->dev);
> > - else
> > - chip_id = id->driver_data;
> > + ch = i2c_get_match_data(client);
> > +
> > pdata = dev_get_platdata(&client->dev);
> >
> > if (!pdata) {
> > @@ -414,7 +412,7 @@ static int mcp4725_probe(struct i2c_client *client)
> > pdata = &pdata_dt;
> > }
> >
> > - if (chip_id == MCP4725 && pdata->use_vref) {
> > + if (ch->ext_info == mcp4725_ext_info && pdata->use_vref) {
>
> I think this will end up being hard to maintain.
>
> Please define a mcp4725_chip_info structure and put the channels in there
> + a flag that says if the device in question supports an external
> reference.
Agreed.
>
> > dev_err(&client->dev,
> > "external reference is unavailable on MCP4725");
> > return -EINVAL;
> > @@ -455,12 +453,12 @@ static int mcp4725_probe(struct i2c_client
> > *client)
> >
> > indio_dev->name = id->name;
> > indio_dev->info = &mcp4725_info;
> > - indio_dev->channels = &mcp472x_channel[chip_id];
> > + indio_dev->channels = ch;
> > indio_dev->num_channels = 1;
> > indio_dev->modes = INDIO_DIRECT_MODE;
> >
> > /* read current DAC value and settings */
> > - err = i2c_master_recv(client, inbuf, chip_id == MCP4725 ? 3 : 4);
> > + err = i2c_master_recv(client, inbuf, ch->ext_info ==
> > +mcp4725_ext_info ? 3 : 4);
> >
> > if (err < 0) {
> > dev_err(&client->dev, "failed to read DAC value"); @@ -470,10
> > +468,10 @@ static int mcp4725_probe(struct i2c_client *client)
> > data->powerdown = pd > 0;
> > data->powerdown_mode = pd ? pd - 1 : 2; /* largest resistor to gnd */
> > data->dac_value = (inbuf[1] << 4) | (inbuf[2] >> 4);
> > - if (chip_id == MCP4726)
> > + if (ch->ext_info == mcp4726_ext_info)
> > ref = (inbuf[3] >> 3) & 0x3;
>
> Add a flag to the chip_info structure for whatever we are controlling here
> and then have if (info->x_is_supported)
OK.
> ...
>
>
> >
> > - if (chip_id == MCP4726 && ref != data->ref_mode) {
> > + if (ch->ext_info == mcp4726_ext_info && ref != data->ref_mode) {
>
> I think this can also use the vref bool I suggest adding above.
OK.
Cheers,
Biju
>
> > dev_info(&client->dev,
> > "voltage reference mode differs (conf: %u, eeprom: %u),
> setting %u",
> > data->ref_mode, ref, data->ref_mode); @@ -511,8 +509,8
> @@ static
> > void mcp4725_remove(struct i2c_client *client) }
> >
> > static const struct i2c_device_id mcp4725_id[] = {
> > - { "mcp4725", MCP4725 },
> > - { "mcp4726", MCP4726 },
> > + { "mcp4725", (kernel_ulong_t)&mcp472x_channel[MCP4725] },
> > + { "mcp4726", (kernel_ulong_t)&mcp472x_channel[MCP4726] },
> > { }
> > };
> > MODULE_DEVICE_TABLE(i2c, mcp4725_id); @@ -520,11 +518,11 @@
> > MODULE_DEVICE_TABLE(i2c, mcp4725_id); static const struct
> > of_device_id mcp4725_of_match[] = {
> > {
> > .compatible = "microchip,mcp4725",
> > - .data = (void *)MCP4725
> > + .data = &mcp472x_channel[MCP4725]
> > },
> > {
> > .compatible = "microchip,mcp4726",
> > - .data = (void *)MCP4726
> > + .data = &mcp472x_channel[MCP4726]
> > },
> > { }
> > };
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-29 14:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-18 17:49 [PATCH v2 0/2] Use i2c_get_match_data() for mcp4725 DAC Biju Das
2023-08-18 17:49 ` [PATCH v2 1/2] iio: dac: mcp4725: Replace variable 'id' from struct mcp4725_data Biju Das
2023-08-18 17:49 ` [PATCH v2 2/2] iio: dac: mcp4725: Use i2c_get_match_data() Biju Das
2023-08-28 12:45 ` Jonathan Cameron
2023-08-29 14:35 ` Biju Das
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.