On 05/08/2022 09:12, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 05/08/2022 09:34, Conor.Dooley@xxxxxxxxxxxxx wrote: >> On 05/08/2022 07:49, Krzysztof Kozlowski wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On 05/08/2022 07:30, Naga Sureshkumar Relli wrote: >>>> diff --git a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml >>>> index a47d4923b51b..84d32c1a4d60 100644 >>>> --- a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml >>>> +++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml >>>> @@ -18,10 +18,12 @@ allOf: >>>> >>>> properties: >>>> compatible: >>>> - enum: >>>> - - microchip,mpfs-spi >>>> - - microchip,mpfs-qspi >>>> - - microchip,coreqspi-rtl-v2 # FPGA QSPI >>>> + oneOf: >>>> + - items: >>>> + - const: microchip,mpfs-qspi >>>> + - const: microchip,coreqspi-rtl-v2 >>> >>> Eh, this does not make sense after looking at your driver... >> >> What is wrong with explicitly binding the driver to both of the >> compatible strings? The "hard" peripheral in the SoC part of the >> FPGA is a superset of version 2 of the coreQSPI IP so the fallback >> used in the binding here makes sense to me. coreQSPI can be >> instantiated in the FPGA fabric and used there, so it needs a >> compatible of its own. >> >> That brings me back to the original point question, why not >> explicitly bind the driver to both of the compatible strings it >> is known to work for? > > There is nothing particularly bad with matching to both of compatibles. > It is valid code. There are however questions/issues with that: > > 1. It is redundant. I did not look too much at the driver, but none of > the of_device_id entries have some driver data to differentiate, > therefore - for the driver - the devices are identical. If they are > identical and according to binding compatible, use less code and just > one compatible. Right. Then the binding is correct and the driver should only bind against "microchip,coreqspi-rtl-v2". > > 2. Otherwise, maybe the devices are not actually fully compatible? > That's the second problem. If one writes binding like that and codes it > in driver differently, it looks like it was not investigated really and > I ask questions... > > Best regards, > Krzysztof
