| Message ID | 189b903e939e15d98d198db60e2ca0619b116870.1706441008.git.lorenzo@kernel.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | add support for ASM330LHHXG1 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Sun, 28 Jan 2024 12:25:45 +0100 Lorenzo Bianconi <lorenzo@kernel.org> wrote: > Add device bindings for asm330lhhxg1 IMU sensor. > Use asm330lhhx as fallback device for asm330lhhxg1 since it implements > all the features currently supported by asm330lhhxg1. If there are no other changes, I can amend this text whilst applying. Something lie: The asm330lhhx supports a subset of the features and functionality provided by the asm330lhhxg1 via identical interfaces and so is a suitable fallback compatible. As normal for bindings we shouldn't talk about the driver though what we really care about here is that the driver will never support functionality for the fallback part that doesn't apply for the more advanced part (because there isn't any!) > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > index 28b667a9cb76..7ba3de66a2e1 100644 > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > @@ -49,6 +49,9 @@ properties: > - items: > - const: st,asm330lhb > - const: st,asm330lhh > + - items: > + - const: st,asm330lhhxg1 > + - const: st,asm330lhhx > > reg: > maxItems: 1
> On Sun, 28 Jan 2024 12:25:45 +0100 > Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > Add device bindings for asm330lhhxg1 IMU sensor. > > Use asm330lhhx as fallback device for asm330lhhxg1 since it implements > > all the features currently supported by asm330lhhxg1. > > If there are no other changes, I can amend this text whilst applying. > Something lie: > > The asm330lhhx supports a subset of the features and functionality > provided by the asm330lhhxg1 via identical interfaces and so is a > suitable fallback compatible. I think it is fine, thx Jonathan. Regards, Lorenzo > > As normal for bindings we shouldn't talk about the driver though > what we really care about here is that the driver will never support > functionality for the fallback part that doesn't apply for the > more advanced part (because there isn't any!) > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > index 28b667a9cb76..7ba3de66a2e1 100644 > > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > @@ -49,6 +49,9 @@ properties: > > - items: > > - const: st,asm330lhb > > - const: st,asm330lhh > > + - items: > > + - const: st,asm330lhhxg1 > > + - const: st,asm330lhhx > > > > reg: > > maxItems: 1 >
On 28/01/2024 12:25, Lorenzo Bianconi wrote: > Add device bindings for asm330lhhxg1 IMU sensor. > Use asm330lhhx as fallback device for asm330lhhxg1 since it implements > all the features currently supported by asm330lhhxg1. > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > index 28b667a9cb76..7ba3de66a2e1 100644 > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > @@ -49,6 +49,9 @@ properties: > - items: > - const: st,asm330lhb > - const: st,asm330lhh > + - items: > + - const: st,asm330lhhxg1 > + - const: st,asm330lhhx lhhx is compatible with st,lsm6dsr, so some explanation would be useful why it is not included here. Best regards, Krzysztof
> On 28/01/2024 12:25, Lorenzo Bianconi wrote: > > Add device bindings for asm330lhhxg1 IMU sensor. > > Use asm330lhhx as fallback device for asm330lhhxg1 since it implements > > all the features currently supported by asm330lhhxg1. > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > index 28b667a9cb76..7ba3de66a2e1 100644 > > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > @@ -49,6 +49,9 @@ properties: > > - items: > > - const: st,asm330lhb > > - const: st,asm330lhh > > + - items: > > + - const: st,asm330lhhxg1 > > + - const: st,asm330lhhx > > lhhx is compatible with st,lsm6dsr, so some explanation would be useful > why it is not included here. > > Best regards, > Krzysztof > Do you mean something like: diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml index 7ba3de66a2e1..92feba6e2427 100644 --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml @@ -35,6 +35,7 @@ properties: - st,lsm6dsv - st,lsm6dso16is - items: + - const: st,asm330lhhxg1 - const: st,asm330lhhx - const: st,lsm6dsr - items: Regards, Lorenzo
On 29/01/2024 14:30, Lorenzo Bianconi wrote: >> On 28/01/2024 12:25, Lorenzo Bianconi wrote: >>> Add device bindings for asm330lhhxg1 IMU sensor. >>> Use asm330lhhx as fallback device for asm330lhhxg1 since it implements >>> all the features currently supported by asm330lhhxg1. >>> >>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> >>> --- >>> Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml >>> index 28b667a9cb76..7ba3de66a2e1 100644 >>> --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml >>> +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml >>> @@ -49,6 +49,9 @@ properties: >>> - items: >>> - const: st,asm330lhb >>> - const: st,asm330lhh >>> + - items: >>> + - const: st,asm330lhhxg1 >>> + - const: st,asm330lhhx >> >> lhhx is compatible with st,lsm6dsr, so some explanation would be useful >> why it is not included here. >> >> Best regards, >> Krzysztof >> > > Do you mean something like: > I meant either explanation in commit msg or adding lsm6dsr fallback. Best regards, Krzysztof
> On 29/01/2024 14:30, Lorenzo Bianconi wrote: > >> On 28/01/2024 12:25, Lorenzo Bianconi wrote: > >>> Add device bindings for asm330lhhxg1 IMU sensor. > >>> Use asm330lhhx as fallback device for asm330lhhxg1 since it implements > >>> all the features currently supported by asm330lhhxg1. > >>> > >>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > >>> --- > >>> Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++ > >>> 1 file changed, 3 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > >>> index 28b667a9cb76..7ba3de66a2e1 100644 > >>> --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > >>> +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > >>> @@ -49,6 +49,9 @@ properties: > >>> - items: > >>> - const: st,asm330lhb > >>> - const: st,asm330lhh > >>> + - items: > >>> + - const: st,asm330lhhxg1 > >>> + - const: st,asm330lhhx > >> > >> lhhx is compatible with st,lsm6dsr, so some explanation would be useful > >> why it is not included here. > >> > >> Best regards, > >> Krzysztof > >> > > > > Do you mean something like: > > > > I meant either explanation in commit msg or adding lsm6dsr fallback. > > Best regards, > Krzysztof > I think we can use lsm6dsr as fallback instead of asm330lhhx (for st_lsm6dsx there is no difference), I have just used asm330lhhx since both asm330lhhx and asm330lhhxg1 are automotive sensors. Regards, Lorenzo
On Mon, Jan 29, 2024 at 02:30:27PM +0100, Lorenzo Bianconi wrote: > > On 28/01/2024 12:25, Lorenzo Bianconi wrote: > > > Add device bindings for asm330lhhxg1 IMU sensor. > > > Use asm330lhhx as fallback device for asm330lhhxg1 since it implements > > > all the features currently supported by asm330lhhxg1. > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > --- > > > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > index 28b667a9cb76..7ba3de66a2e1 100644 > > > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > @@ -49,6 +49,9 @@ properties: > > > - items: > > > - const: st,asm330lhb > > > - const: st,asm330lhh > > > + - items: > > > + - const: st,asm330lhhxg1 > > > + - const: st,asm330lhhx > > > > lhhx is compatible with st,lsm6dsr, so some explanation would be useful > > why it is not included here. > > > > Best regards, > > Krzysztof > > > > Do you mean something like: > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > index 7ba3de66a2e1..92feba6e2427 100644 > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > @@ -35,6 +35,7 @@ properties: > - st,lsm6dsv > - st,lsm6dso16is > - items: > + - const: st,asm330lhhxg1 > - const: st,asm330lhhx > - const: st,lsm6dsr No, that's not right because you just changed the existing user. You need to explain why "st,lsm6dsr" is not needed as a fallback or you need your original change with "st,lsm6dsr" added on the end. (And group it next to the original entry, not at the end.) The question to ask is whether software that only understands what "st,lsm6dsr" is will work with this newer device. Rob
diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml index 28b667a9cb76..7ba3de66a2e1 100644 --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml @@ -49,6 +49,9 @@ properties: - items: - const: st,asm330lhb - const: st,asm330lhh + - items: + - const: st,asm330lhhxg1 + - const: st,asm330lhhx reg: maxItems: 1
Add device bindings for asm330lhhxg1 IMU sensor. Use asm330lhhx as fallback device for asm330lhhxg1 since it implements all the features currently supported by asm330lhhxg1. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++ 1 file changed, 3 insertions(+)