| Message ID | 441d9babc7042892350a58ba4300df4761876ae3.1701334008.git.michal.simek@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dt-bindings: hwmon: Increase max number of io-channels | expand |
On Thu, Nov 30, 2023 at 09:46:54AM +0100, Michal Simek wrote: > arch/arm64/boot/dts/xilinx/zynqmp-sm-k26-revA.dts is defining 30 channels > that's why increase it. > But AMS binding defines 51 channels > Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > that's why increase number to 51. Please write commit messages for bindings in terms of how they relate to some hardware. In this case I think it would be sufficient to say that the Analog Monitoring system has 51 channels, the opening sentence about the dts as justification provides no actionable information. Acked-by: Conor Dooley <conor.dooley@microchip.com> Cheers, Conor. > Signed-off-by: Michal Simek <michal.simek@amd.com> > --- > > Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml > index e5b24782f448..be5c7d4579bb 100644 > --- a/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml > +++ b/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml > @@ -19,7 +19,7 @@ properties: > > io-channels: > minItems: 1 > - maxItems: 8 # Should be enough > + maxItems: 51 # Should be enough > description: > > List of phandles to ADC channels to read the monitoring values > > -- > 2.36.1 >
On 11/30/23 15:58, Conor Dooley wrote: > On Thu, Nov 30, 2023 at 09:46:54AM +0100, Michal Simek wrote: >> arch/arm64/boot/dts/xilinx/zynqmp-sm-k26-revA.dts is defining 30 channels >> that's why increase it. >> But AMS binding defines 51 channels >> Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml >> that's why increase number to 51. > > Please write commit messages for bindings in terms of how they relate to > some hardware. In this case I think it would be sufficient to say that > the Analog Monitoring system has 51 channels, the opening sentence about > the dts as justification provides no actionable information. > Acked-by: Conor Dooley <conor.dooley@microchip.com> v2 sent. Thanks, Michal
On Fri, Dec 01, 2023 at 09:35:27AM +0100, Michal Simek wrote: > > > On 11/30/23 15:58, Conor Dooley wrote: > > On Thu, Nov 30, 2023 at 09:46:54AM +0100, Michal Simek wrote: > > > arch/arm64/boot/dts/xilinx/zynqmp-sm-k26-revA.dts is defining 30 channels > > > that's why increase it. > > > But AMS binding defines 51 channels > > > Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml > > > that's why increase number to 51. > > > > Please write commit messages for bindings in terms of how they relate to > > some hardware. In this case I think it would be sufficient to say that > > the Analog Monitoring system has 51 channels, the opening sentence about > > the dts as justification provides no actionable information. > > Acked-by: Conor Dooley <conor.dooley@microchip.com> > > v2 sent. I wasn't requesting a v2 FWIW. "$file uses 30 channels" rather than rephrasing it in terms of the hardware actually having 51 channels just annoyed me, that's all.
On 12/1/23 17:07, Conor Dooley wrote: > On Fri, Dec 01, 2023 at 09:35:27AM +0100, Michal Simek wrote: >> >> >> On 11/30/23 15:58, Conor Dooley wrote: >>> On Thu, Nov 30, 2023 at 09:46:54AM +0100, Michal Simek wrote: >>>> arch/arm64/boot/dts/xilinx/zynqmp-sm-k26-revA.dts is defining 30 channels >>>> that's why increase it. >>>> But AMS binding defines 51 channels >>>> Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml >>>> that's why increase number to 51. >>> >>> Please write commit messages for bindings in terms of how they relate to >>> some hardware. In this case I think it would be sufficient to say that >>> the Analog Monitoring system has 51 channels, the opening sentence about >>> the dts as justification provides no actionable information. >>> Acked-by: Conor Dooley <conor.dooley@microchip.com> >> >> v2 sent. > > I wasn't requesting a v2 FWIW. "$file uses 30 channels" rather than > rephrasing it in terms of the hardware actually having 51 channels just > annoyed me, that's all. I know and I agreed that commit message can be improved that's why sent v2. Thanks, Michal
diff --git a/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml index e5b24782f448..be5c7d4579bb 100644 --- a/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml +++ b/Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml @@ -19,7 +19,7 @@ properties: io-channels: minItems: 1 - maxItems: 8 # Should be enough + maxItems: 51 # Should be enough description: > List of phandles to ADC channels to read the monitoring values
arch/arm64/boot/dts/xilinx/zynqmp-sm-k26-revA.dts is defining 30 channels that's why increase it. But AMS binding defines 51 channels Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml that's why increase number to 51. Signed-off-by: Michal Simek <michal.simek@amd.com> --- Documentation/devicetree/bindings/hwmon/iio-hwmon.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)