On 11/04/2024 16:35, Binbin Zhou wrote: > On Thu, Apr 11, 2024 at 5:07 PM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 11/04/2024 13:01, Binbin Zhou wrote: >>> On Thu, Apr 11, 2024 at 4:26 PM Krzysztof Kozlowski >>> <krzysztof.kozlowski@xxxxxxxxxx> wrote: >>>> >>>> On 11/04/2024 11:16, Binbin Zhou wrote: >>>>> Add Loongson PWM controller binding with DT schema format using >>>>> json-schema. >>>>> >>>>> Signed-off-by: Binbin Zhou <zhoubinbin@xxxxxxxxxxx> >>>> >>>> >>>>> +properties: >>>>> + compatible: >>>>> + oneOf: >>>>> + - const: loongson,ls7a-pwm >>>>> + - items: >>>>> + - enum: >>>>> + - loongson,ls2k0500-pwm >>>>> + - loongson,ls2k1000-pwm >>>>> + - loongson,ls2k2000-pwm >>>>> + - const: loongson,ls7a-pwm >>>>> + >>>>> + reg: >>>>> + maxItems: 1 >>>>> + >>>>> + interrupts: >>>>> + maxItems: 1 >>>>> + >>>>> + clocks: >>>>> + maxItems: 1 >>>>> + >>>>> + '#pwm-cells': >>>>> + description: >>>>> + The first cell must have a value of 0, which specifies the PWM output signal; >>>> >>>> If you have always the same value in PWM phandle, why encoding it in the >>>> phandle in the first place? What's the benefit of passing 0? >>> >>> Hi Krzysztof: >>> >>> My thoughts are: >>> First of all, our pwm has only one output signal, so it can only be 0. >>> Also, as you know from the pwm xlate function, the first cell is the >>> pwm index, so I fixed it to be 0 here. >>> >>> The xlate function: >>> https://elixir.bootlin.com/linux/v6.8/source/drivers/pwm/core.c#L106 >> >> You refer for xlate for PWM with three cells. You do not have three >> cells, as you have only on signal, so why insisting on using other >> xlate? Do you do the same for clocks? Or resets? >> >> I don't think you use appropriate argument in this discussion. We talk >> about hardware and your argument "I don't want to use my own xlate in >> the driver" is about driver. >> > Hi Krzysztof: > > Thanks for your comments. > > Emm... Indeed, I used to think about it from the driver's perspective. > From the binding perspective, two cells really should be more appropriate. > I try to make the following changes in the next version patchset: > > '#pwm-cells': > description: > The first cell is the period in nanoseconds; > The second cell flag supported by this binding is PWM_POLARITY_INVERTED. > const: 2 > > Accordingly, the custom xlate function will be used in the driver. If your other, upcoming variants had more PWM outputs, then I would find reasonable keeping cells=3 to have one approach for all of them. But I guess that's not the case here. Best regards, Krzysztof
