On 15/11/2022 14:59, Vladimir Oltean wrote: > On Tue, Nov 15, 2022 at 02:46:21PM +0100, Krzysztof Kozlowski wrote: >>> +$id: http://devicetree.org/schemas/spi/fsl,spi-fsl-dspi.yaml >> >> Why second "fsl" in file name? It does not patch compatibles and >> duplicates the vendor. We do not have compatibles "nxp,imx6-nxp". > > Ok, which file name would be good then? There are 9 different (all SoC > specific) compatible strings, surely the convention of naming the file > after a compatible string has some limitations... If all DSPI blocks fit here, then maybe: fsl,dspi.yaml fsl,spi-dspi.yaml is also a bit redundant. > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml >>> + >>> +title: Freescale DSPI Controller >>> + >>> +maintainers: >>> + - Vladimir Oltean <olteanv@xxxxxxxxx> >>> + >>> +allOf: >>> + - $ref: "spi-controller.yaml#" >> >> Drop quotes. >> >>> + >>> +properties: >>> + compatible: >>> + description: >>> + Some integrations can have a single compatible string containing their >>> + SoC name (LS1012A, LS1021A, ...). Others require their SoC compatible >>> + string, plus a fallback compatible string (either on LS1021A or on >>> + LS2085A). >> >> Why? The fsl,ls1012a-dspi device is either compatible with >> fsl,ls1021a-v1.0-dspi or not. It cannot be both - compatible and not >> compatible. > > LS1012A is compatible with LS1021A to the extent that it works when > treated like a LS1021A. LS1012A has a FIFO size of 8 SPI words, LS1021A > of just 4. Treating it like LS1021A means roughly half the performance, > but it still works. > > I didn't invent any of this. When I took over the driver, there were > device trees like this all over the place: > > dspi: spi@2100000 { > compatible = "fsl,ls1012a-dspi", "fsl,ls1021a-v1.0-dspi"; Which looks ok... > #address-cells = <1>; > #size-cells = <0>; > reg = <0x0 0x2100000 0x0 0x10000>; > interrupts = <0 64 IRQ_TYPE_LEVEL_HIGH>; > clock-names = "dspi"; > clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL > QORIQ_CLK_PLL_DIV(1)>; > spi-num-chipselects = <5>; > big-endian; > status = "disabled"; > }; > > but the Linux driver pre-~5.7 always relied on the fallback compatible > string (LS1021A in this case). I'm working with what's out in the field, > haven't changed a thing there. The driver matters less (except ABI), but anyway it confirms the case - fallback is expected always. Why the fallback should be removed if the devices are compatible (including halved performance)? > >>> + oneOf: >>> + - enum: >>> + - fsl,ls1012a-dspi >>> + - fsl,ls1021a-v1.0-dspi >>> + - fsl,ls1028a-dspi >>> + - fsl,ls2085a-dspi >>> + - fsl,lx2160a-dspi >>> + - fsl,vf610-dspi >>> + - items: >>> + - enum: >>> + - fsl,ls1012a-dspi >>> + - fsl,ls1028a-dspi >>> + - fsl,ls1043a-dspi >>> + - fsl,ls1046a-dspi >>> + - fsl,ls1088a-dspi >>> + - const: fsl,ls1021a-v1.0-dspi >>> + - items: >>> + - enum: >>> + - fsl,ls2080a-dspi >>> + - fsl,lx2160a-dspi >>> + - const: fsl,ls2085a-dspi >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + clocks: >>> + maxItems: 1 >>> + >>> + clock-names: >>> + items: >>> + - const: dspi >>> + >>> + dmas: >>> + maxItems: 2 >>> + >>> + dma-names: >>> + items: >>> + - const: tx >>> + - const: rx >>> + >>> + spi-num-chipselects: >> >> Would be nice to deprecated it in separate patches. There is num-cs >> property. > > Will add this on my TODO list. Right now I'm just converting what exists. Sure. Best regards, Krzysztof
