Re: [PATCH v7 1/4] dt-bindings: pwm: Add OpenCores PWM module

[William Qiu <william.qiu@starfivetech.com>]

On 2023/11/23 1:36, Conor Dooley wrote:
> On Wed, Nov 22, 2023 at 03:03:36PM +0800, William Qiu wrote:
>> 
>> 
>> On 2023/11/14 4:17, Conor Dooley wrote:
>> > On Mon, Nov 13, 2023 at 09:07:15PM +0100, Krzysztof Kozlowski wrote:
>> >> On 13/11/2023 10:42, William Qiu wrote:
>> >> > Will update.
>> >> >>> +
>> >> >>> +allOf:
>> >> >>> +  - $ref: pwm.yaml#
>> >> >>> +
>> >> >>> +properties:
>> >> >>> +  compatible:
>> >> >>> +    oneOf:
>> >> >>> +      - items:
>> >> >>> +          - enum:
>> >> >>> +              - starfive,jh7100-pwm
>> >> >>> +              - starfive,jh7110-pwm
>> >> >>> +          - const: opencores,pwm
>> >> >>
>> >> >> That's a very, very generic compatible. Are you sure, 100% sure, that
>> >> >> all designs from OpenCores from now till next 100 years will be 100%
>> >> >> compatible?
>> >> >>
>> >> > My description is not accurate enough, this is OpenCores PTC IP, and PWM
>> >> > is one of those modes, so it might be better to replace compatible with
>> >> > "opencores, ptc-pwm"
>> >> > 
>> >> > What do you think?
>> >> 
>> >> Sorry, maybe this answers maybe doesn't. What is "PTC"?
>> > 
>> > "pwm timer counter". AFAIU, the IP can be configured to provide all 3.
>> > I think that William pointed out on an earlier revision that they have
>> > only implemented the pwm on their hardware.
>> > I don't think putting in "ptc" is a sufficient differentiator though, as
>> > clearly there could be several different versions of "ptc-pwm" that have
>> > the same concern about "all designs from OpenCores for now till the next
>> > 100 years" being compatible.
> 
> Perhaps noting what "ptc" stands for in the description field would be a
> good idea.
> 
I will add.
>> After discussion and review of materials, we plan to use "opencores,ptc-pwm-v1"
>> as this version of compatible, so that it can also be compatible in the future.
>> 
>> What do you think?
> 
> Do we know that it is actually "v1" of the IP? I would suggest using the
> version that actually matches the version of the IP that you are using
> in your SoC.
> 
> Thanks,
> Conor.

There is no version list on their official website, so it is not certain whether
it is v1, but at least the driver is the first version.

What do you think is the best way?

Thanks,
William