On 28/08/2024 02:43, David Leonard wrote: > On Tue, 27 Aug 2024, Krzysztof Kozlowski wrote: > >> On 27/08/2024 18:51, David Leonard wrote: >>>>> +properties: >>>>> + compatible: >>>>> + const: fsl,ls1012a-pinctrl >>>>> + >>>>> + reg: >>>>> + description: Specifies the base address of the PMUXCR0 register. >>>>> + maxItems: 2 >>>> >>>> Instead list and describe the items. >>> >>> Changed to >>> >>> reg: >>> items: >>> - description: Physical base address of the PMUXCR0 register. >>> - description: Size of the PMUXCR0 register (4). >>> >>> Is this what you meant? >> >> Almost, second reg is not a size. You claim there are two IO address >> spaces. Each address space contains base address and size. Look at other >> bindings how they do it. > > Previously, dt_binding_check was giving me a warning that 'maxItems' needed > to be supplied. Which confused me because I thought of reg as an opaque subspace No. > descriptor, but I was just trying to placate the checks. After tool upgrades, > that warning doesn't appear any more. > > So the neatest documentation would be: > > reg: > description: Specifies the address of the PMUXCR0 register. No. Please go through example-schema. Or any other binding... If you find any binding with above syntax, let me know. Instead: maxItems: 1. > >>>>> + big-endian: >>>>> + description: If present, the PMUXCR0 register is implemented in big-endian. >>>> >>>> Why is this here? Either it is or it is not? >>> >>> You're right. Changed to >>> >>> big-endian: true >>> >>> (This also lead to some code simplification) >> >> OK, but I still wonder why is it here. Without it the hardware will work >> in little-endian? > > Well, it's here firstly because I was trying to follow a perceived convention in > dts/freescale/fsl,ls1012a.dtsi. That DT uses big-endian in child I am confused. Are you adding new device or converting existing bindings? > nodes of /soc that match up with memory map tables from the datasheet. > (Not only do different and adjacent parts of the register map have > opposing endianess, some register layouts also seem to be reversed > bitwise, others bytewise.) > > The second reason is practical/dodgy. The pinctrl driver should logically > be a child of the scfg node, but the syscon driver doesn't populate its > child nodes. To get the pinctrl driver to work meant making it a sibling So fix driver... > node with an unsatisfactory overlap with the scfg's address region > 0x1570000+0x10000. (No driver binds to "fsl,ls1012a-scfg".) > Several big-endian properties were added unnecessarily and are now being removed. You cannot provide me answer whether this hardware is little/big endian, so it also feels unnecessary. Best regards, Krzysztof
