* [PATCH] iio: st_pressure_spi: Add missing entries SPI to device ID table
@ 2021-09-27 13:41 Mark Brown
2021-09-30 16:38 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2021-09-27 13:41 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen; +Cc: linux-iio, Mark Brown
Currently autoloading for SPI devices does not use the DT ID table, it uses
SPI modalises. Supporting OF modalises is going to be difficult if not
impractical, an attempt was made but has been reverted, so ensure that
module autoloading works for this driver by adding SPI IDs for parts that
only have a compatible listed.
Fixes: 96c8395e2166 ("spi: Revert modalias changes")
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/iio/pressure/st_pressure_spi.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
index 9b2523c5bc94..d6fc954e28f8 100644
--- a/drivers/iio/pressure/st_pressure_spi.c
+++ b/drivers/iio/pressure/st_pressure_spi.c
@@ -97,6 +97,10 @@ static const struct spi_device_id st_press_id_table[] = {
{ LPS33HW_PRESS_DEV_NAME },
{ LPS35HW_PRESS_DEV_NAME },
{ LPS22HH_PRESS_DEV_NAME },
+ { "lps001wp-press" },
+ { "lps25h-press", },
+ { "lps331ap-press" },
+ { "lps22hb-press" },
{},
};
MODULE_DEVICE_TABLE(spi, st_press_id_table);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: st_pressure_spi: Add missing entries SPI to device ID table
2021-09-27 13:41 [PATCH] iio: st_pressure_spi: Add missing entries SPI to device ID table Mark Brown
@ 2021-09-30 16:38 ` Jonathan Cameron
2021-09-30 18:35 ` Denis CIOCCA
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2021-09-30 16:38 UTC (permalink / raw)
To: Mark Brown; +Cc: Lars-Peter Clausen, linux-iio, Denis Ciocca
On Mon, 27 Sep 2021 14:41:53 +0100
Mark Brown <broonie@kernel.org> wrote:
> Currently autoloading for SPI devices does not use the DT ID table, it uses
> SPI modalises. Supporting OF modalises is going to be difficult if not
> impractical, an attempt was made but has been reverted, so ensure that
> module autoloading works for this driver by adding SPI IDs for parts that
> only have a compatible listed.
>
> Fixes: 96c8395e2166 ("spi: Revert modalias changes")
> Signed-off-by: Mark Brown <broonie@kernel.org>
Whilst these IDs are deprecated, we should at least be consistent that they either
work or not rather than current situation.
+CC Denis as driver author. I'll let it sit on list a little longer so Denis can
take a look.
Thanks
Jonathan
> ---
> drivers/iio/pressure/st_pressure_spi.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
> index 9b2523c5bc94..d6fc954e28f8 100644
> --- a/drivers/iio/pressure/st_pressure_spi.c
> +++ b/drivers/iio/pressure/st_pressure_spi.c
> @@ -97,6 +97,10 @@ static const struct spi_device_id st_press_id_table[] = {
> { LPS33HW_PRESS_DEV_NAME },
> { LPS35HW_PRESS_DEV_NAME },
> { LPS22HH_PRESS_DEV_NAME },
> + { "lps001wp-press" },
> + { "lps25h-press", },
> + { "lps331ap-press" },
> + { "lps22hb-press" },
> {},
> };
> MODULE_DEVICE_TABLE(spi, st_press_id_table);
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] iio: st_pressure_spi: Add missing entries SPI to device ID table
2021-09-30 16:38 ` Jonathan Cameron
@ 2021-09-30 18:35 ` Denis CIOCCA
2021-10-01 12:19 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Denis CIOCCA @ 2021-09-30 18:35 UTC (permalink / raw)
To: Jonathan Cameron, Mark Brown; +Cc: Lars-Peter Clausen, linux-iio
Hello Jonathan, Mark,
I am not very familiar with how much the kernel would like to keep 'probing id' consistent. I perfectly understand the value of doing this (maintain ID compatibility) but I also see increase confusion in maintaining half in a way and half in another.
I personally think that we should drop the '-press' thing for all the devices since they all are single-chip (meaning that the name used identify univocally that is a pressure sensor).
If you think that compatibility is more important here, I think the patch is fine but this should be done in the i2c part as well so that it's at least congruent withing the driver.
Let me know what you think.
Thanks,
Denis
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Thursday, September 30, 2021 9:39 AM
> To: Mark Brown <broonie@kernel.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>; linux-iio@vger.kernel.org; Denis
> CIOCCA <denis.ciocca@st.com>
> Subject: Re: [PATCH] iio: st_pressure_spi: Add missing entries SPI to device ID
> table
>
> On Mon, 27 Sep 2021 14:41:53 +0100
> Mark Brown <broonie@kernel.org> wrote:
>
> > Currently autoloading for SPI devices does not use the DT ID table, it
> > uses SPI modalises. Supporting OF modalises is going to be difficult
> > if not impractical, an attempt was made but has been reverted, so
> > ensure that module autoloading works for this driver by adding SPI IDs
> > for parts that only have a compatible listed.
> >
> > Fixes: 96c8395e2166 ("spi: Revert modalias changes")
> > Signed-off-by: Mark Brown <broonie@kernel.org>
>
> Whilst these IDs are deprecated, we should at least be consistent that they
> either work or not rather than current situation.
>
> +CC Denis as driver author. I'll let it sit on list a little longer so
> +Denis can
> take a look.
>
> Thanks
>
> Jonathan
>
> > ---
> > drivers/iio/pressure/st_pressure_spi.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/iio/pressure/st_pressure_spi.c
> > b/drivers/iio/pressure/st_pressure_spi.c
> > index 9b2523c5bc94..d6fc954e28f8 100644
> > --- a/drivers/iio/pressure/st_pressure_spi.c
> > +++ b/drivers/iio/pressure/st_pressure_spi.c
> > @@ -97,6 +97,10 @@ static const struct spi_device_id st_press_id_table[]
> = {
> > { LPS33HW_PRESS_DEV_NAME },
> > { LPS35HW_PRESS_DEV_NAME },
> > { LPS22HH_PRESS_DEV_NAME },
> > + { "lps001wp-press" },
> > + { "lps25h-press", },
> > + { "lps331ap-press" },
> > + { "lps22hb-press" },
> > {},
> > };
> > MODULE_DEVICE_TABLE(spi, st_press_id_table);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: st_pressure_spi: Add missing entries SPI to device ID table
2021-09-30 18:35 ` Denis CIOCCA
@ 2021-10-01 12:19 ` Mark Brown
2021-10-07 0:47 ` Denis CIOCCA
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2021-10-01 12:19 UTC (permalink / raw)
To: Denis CIOCCA; +Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio
[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]
On Thu, Sep 30, 2021 at 06:35:23PM +0000, Denis CIOCCA wrote:
> I am not very familiar with how much the kernel would like to keep 'probing id' consistent. I perfectly understand the value of doing this (maintain ID compatibility) but I also see increase confusion in maintaining half in a way and half in another.
The goal is not to maintain compatibility, the goal is to be able to
load the driver as a module on DT systems. For historical reasons SPI
uses the platform device IDs to load modules bound with DT, if there is
no platform ID for a DT ID then userspace won't be able to find and load
the module.
> I personally think that we should drop the '-press' thing for all the devices since they all are single-chip (meaning that the name used identify univocally that is a pressure sensor).
The DT bindings are an ABI, you can't really remove compatibles only
deprecate them.
> If you think that compatibility is more important here, I think the patch is fine but this should be done in the i2c part as well so that it's at least congruent withing the driver.
I2C doesn't have this issue with modaliases so it's not an issue there.
Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns. Doing this makes your messages much
easier to read and reply to.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] iio: st_pressure_spi: Add missing entries SPI to device ID table
2021-10-01 12:19 ` Mark Brown
@ 2021-10-07 0:47 ` Denis CIOCCA
2021-10-07 15:27 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Denis CIOCCA @ 2021-10-07 0:47 UTC (permalink / raw)
To: Mark Brown; +Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio
Hello Mark,
> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Friday, October 1, 2021 5:20 AM
> To: Denis CIOCCA <denis.ciocca@st.com>
> Cc: Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen
> <lars@metafoo.de>; linux-iio@vger.kernel.org
> Subject: Re: [PATCH] iio: st_pressure_spi: Add missing entries SPI to device
> ID table
>
> On Thu, Sep 30, 2021 at 06:35:23PM +0000, Denis CIOCCA wrote:
>
> > I am not very familiar with how much the kernel would like to keep
> 'probing id' consistent. I perfectly understand the value of doing this
> (maintain ID compatibility) but I also see increase confusion in maintaining
> half in a way and half in another.
>
> The goal is not to maintain compatibility, the goal is to be able to
> load the driver as a module on DT systems. For historical reasons SPI
> uses the platform device IDs to load modules bound with DT, if there is
> no platform ID for a DT ID then userspace won't be able to find and load
> the module.
Ok now it is clear. I wasn't aware of that.
In this case it is good to me (I didn't do any testing).
>
> > I personally think that we should drop the '-press' thing for all the devices
> since they all are single-chip (meaning that the name used identify
> univocally that is a pressure sensor).
>
> The DT bindings are an ABI, you can't really remove compatibles only
> deprecate them.
Yeah I guessed this was the case.
>
> > If you think that compatibility is more important here, I think the patch is
> fine but this should be done in the i2c part as well so that it's at least
> congruent withing the driver.
>
> I2C doesn't have this issue with modaliases so it's not an issue there.
Clear.
>
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns. Doing this makes your messages much
> easier to read and reply to.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: st_pressure_spi: Add missing entries SPI to device ID table
2021-10-07 0:47 ` Denis CIOCCA
@ 2021-10-07 15:27 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2021-10-07 15:27 UTC (permalink / raw)
To: Denis CIOCCA; +Cc: Mark Brown, Lars-Peter Clausen, linux-iio
On Thu, 7 Oct 2021 00:47:56 +0000
Denis CIOCCA <denis.ciocca@st.com> wrote:
> Hello Mark,
>
> > -----Original Message-----
> > From: Mark Brown <broonie@kernel.org>
> > Sent: Friday, October 1, 2021 5:20 AM
> > To: Denis CIOCCA <denis.ciocca@st.com>
> > Cc: Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen
> > <lars@metafoo.de>; linux-iio@vger.kernel.org
> > Subject: Re: [PATCH] iio: st_pressure_spi: Add missing entries SPI to device
> > ID table
> >
> > On Thu, Sep 30, 2021 at 06:35:23PM +0000, Denis CIOCCA wrote:
> >
> > > I am not very familiar with how much the kernel would like to keep
> > 'probing id' consistent. I perfectly understand the value of doing this
> > (maintain ID compatibility) but I also see increase confusion in maintaining
> > half in a way and half in another.
> >
> > The goal is not to maintain compatibility, the goal is to be able to
> > load the driver as a module on DT systems. For historical reasons SPI
> > uses the platform device IDs to load modules bound with DT, if there is
> > no platform ID for a DT ID then userspace won't be able to find and load
> > the module.
>
> Ok now it is clear. I wasn't aware of that.
> In this case it is good to me (I didn't do any testing).
Applied to the fixes-togreg branch of iio.git. I haven't explicitly marked
it for stable but it may make sense to backport at somepoint.
Thanks,
Jonathan
>
> >
> > > I personally think that we should drop the '-press' thing for all the devices
> > since they all are single-chip (meaning that the name used identify
> > univocally that is a pressure sensor).
> >
> > The DT bindings are an ABI, you can't really remove compatibles only
> > deprecate them.
>
> Yeah I guessed this was the case.
>
> >
> > > If you think that compatibility is more important here, I think the patch is
> > fine but this should be done in the i2c part as well so that it's at least
> > congruent withing the driver.
> >
> > I2C doesn't have this issue with modaliases so it's not an issue there.
>
> Clear.
>
> >
> > Please fix your mail client to word wrap within paragraphs at something
> > substantially less than 80 columns. Doing this makes your messages much
> > easier to read and reply to.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-07 15:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-27 13:41 [PATCH] iio: st_pressure_spi: Add missing entries SPI to device ID table Mark Brown
2021-09-30 16:38 ` Jonathan Cameron
2021-09-30 18:35 ` Denis CIOCCA
2021-10-01 12:19 ` Mark Brown
2021-10-07 0:47 ` Denis CIOCCA
2021-10-07 15:27 ` Jonathan Cameron
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.