On 01/09/2022 22:37, Larson, Bradley wrote:
> On 9/1/22 12:20 AM, Krzysztof Kozlowski wrote:
>> On 01/09/2022 02:01, Larson, Bradley wrote:
>>>>> + is implemented by a sub-device reset-controller which accesses
>>>>> + a CS0 control register.
>>>>> +
>>>>> +maintainers:
>>>>> + - Brad Larson <blarson@xxxxxxx>
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + items:
>>>>> + - enum:
>>>>> + - amd,pensando-elbasr
>>>>> +
>>>>> + spi-max-frequency:
>>>>> + description: Maximum SPI frequency of the device in Hz.
>>>> No need for generic descriptions of common properties.
>>> Changed to "spi-max-frequency: true" and moved to end of properties.
>> Then you should rather reference spi-peripheral-props just like other
>> SPI devices.
>
>
> Will look at this dependent on the result of below
>
>
>>>>> +
>>>>> + reg:
>>>>> + maxItems: 1
>>>>> +
>>>>> + '#address-cells':
>>>>> + const: 1
>>>>> +
>>>>> + '#size-cells':
>>>>> + const: 0
>>>>> +
>>>>> + interrupts:
>>>>> + maxItems: 1
>>>>> +
>>>>> +required:
>>>>> + - compatible
>>>>> + - reg
>>>>> + - spi-max-frequency
>>>>> +
>>>>> +patternProperties:
>>>>> + '^reset-controller@[a-f0-9]+$':
>>>>> + $ref: /schemas/reset/amd,pensando-elbasr-reset.yaml
>>>>> +
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> + - |
>>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> +
>>>>> + spi {
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> + num-cs = <4>;
>>>>> +
>>>>> + sysc: system-controller@0 {
>>>>> + compatible = "amd,pensando-elbasr";
>>>>> + reg = <0>;
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> + spi-max-frequency = <12000000>;
>>>>> +
>>>>> + rstc: reset-controller@0 {
>>>>> + compatible = "amd,pensando-elbasr-reset";
>>>>> + reg = <0>;
>>>> What does 0 represent here? A register address within 'elbasr' device?
>>> Removed, I recall a check threw a warning or error without reg.
>>>
>>>> Why do you need a child node for this? Are there other sub-devices and
>>>> your binding is incomplete? Just put '#reset-cells' in the parent.
>>> Without a reset-controller node and booting the function
>>> __of_reset_control_get(...) fails to find a match in the list here
>> That's not actually the answer to the question. There was no concerns
>> whether you need or not reset controller. The question was why do you
>> need a child device instead of elbasr being the reset controller.
>>
>> Your answer does not cover this at all, so again - why do you need a
>> child for this?
>>
>
> If the parent becomes a reset-controller compatible with
> "amd,pensando-elbasr-reset" then the /dev node will not be created
Why /dev node will not be created? How bindings affect having or not
having something in /dev ?
> as there is no match to "amd,pensando-elbasr" for cs0. For cs0 internal
> to linux the reset-controller manages one register bit to hardware reset
> the mmc device. A userspace application opens the device node to manage
> transceiver leds, system leds, reporting temps to host, other reset
> controls, etc. Looking at future requirements there likely will be other
> child devices.
You mean "amd,pensando-elbasr" will instantiate some more devices? Why
you cannot add the binding for them now? This is actually important
because earlier we agreed you remove unit address from children.
>
> Going down this path with one compatible should reset-elbasr.c just be
> deleted and fold it into the parent driver pensando-elbasr.c? Then I
> wonder if it even belongs in drivers/mfd and should just be modified
> and put in drivers/spi.
How is it related to bindings?
Best regards,
Krzysztof
