Re: [PATCH] dt-bindings: pinctrl: pinctrl-single: Define a max count for "pinctrl-single,gpio-range"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/18/24 1:57 PM, Nishanth Menon wrote:
On 12:34-20240618, Rob Herring wrote:

On Tue, 18 Jun 2024 11:51:02 -0500, Nishanth Menon wrote:
"pinctrl-single,gpio-range" allows us to define a dis-contiguous
range of pinctrl registers that can have different mux settings for
GPIO mode of operation. However, the maxItems seem to be set to 1 in
processed schema for some reason. This is incorrect. For example:
arch/arm64/boot/dts/hisilicon/hi6220.dtsi and others have more than
one dis-contiguous range.

Arbitrarily define a max 100 count to override the defaults.

Signed-off-by: Nishanth Menon <nm@xxxxxx>
---
I am not sure if I should call this RFC or not.. and if this is even the
right solution.. I am on 2024.05 dt-schema for this check.

I noticed this when adding gpio-ranges for am62p platform:
https://gist.github.com/nmenon/7019cd2f24be47997640df5db60a7544

It is possible that this is a bug in dt-schema, but I have'nt been able
to track it down either.

behavior seen is the following:
pinctrl-single,gpio-range = <&mcu_pmx_range 0 21 7>;
generates no warning
However,
pinctrl-single,gpio-range = <&mcu_pmx_range 0 21 7>, <&mcu_pmx_range 32 2 7>;

generates "is too long" warning.


  Documentation/devicetree/bindings/pinctrl/pinctrl-single.yaml | 1 +
  1 file changed, 1 insertion(+)


My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/pinctrl-single.yaml: properties:pinctrl-single,gpio-range: {'description': 'Optional list of pin base, nr pins & gpio function', '$ref': '/schemas/types.yaml#/definitions/phandle-array', 'maxItems': 100, 'items': [{'items': [{'description': 'phandle of a gpio-range node'}, {'description': 'pin base'}, {'description': 'number of pins'}, {'description': 'gpio function'}]}]} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#

yes, I had expected the same, but processed schema indicates a maxItems
of 1 for reasons I am unable to make sense of.. will be great to have
some additional eyes:


As the warning says, setting the item count is not needed when the
item list content (and therefore item count) is explicitly defined.
The binding is saying there is a single item of 4 elements.

"pinctrl-single,gpio-range"'s type is a single phandle-array[0]. What
you want is an array of phandle-arrays. The length of each is usually
set by the target of the phandle with its #*-cells property.

This binding is a bit of a mess, the phandle is always a pointer to
a node with the cells length hard-coded to 3. This looks to have been done
to allow the driver to use the function "of_parse_phandle_with_args" which
needs a property name for to find the cell count. But that makes no sense
as the count is always 3, the driver cannot accept any other value. The
driver should have just looped of_get_property() 3 times but wanted to
use the helper. So a silly driver mistake has turned into a binding issue.

We should drop the "pinctrl-single,gpio-range" from the binding and
fix the driver.

Andrew

[0] https://patchwork.kernel.org/project/netdevbpf/patch/20220119015038.2433585-1-robh@xxxxxxxxxx/

https://gist.github.com/nmenon/7019cd2f24be47997640df5db60a7544#file-processed-schema-pinctrl

next-20240617 baseline




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux