On 2016/10/21 15:34, Thierry Reding wrote: > On Fri, Oct 21, 2016 at 09:22:36AM +0200, Thierry Reding wrote: >> On Mon, Oct 10, 2016 at 04:53:39PM -0500, Rob Herring wrote: >>> On Mon, Oct 10, 2016 at 07:05:16PM +0800, Jian Yuan wrote: >>>> From: yuanjian <yuanjian12@xxxxxxxxxxxxx> >>>> >>>> Add PWM driver for the PWM controller found on HiSilicon BVT SOCs, like Hi3519V100, Hi3516CV300, etc. >>>> The PWM controller is primarily in charge of controlling P-Iris lens. >>>> >>>> Reviewed-by: Jiancheng Xue <xuejiancheng@xxxxxxxxxxxxx> >>>> Signed-off-by: Jian Yuan <yuanjian12@xxxxxxxxxxxxx> >>>> --- >>>> Change Log: >>>> v4: >>>> Add #pwm-cells in the bindings document. >>>> v3: >>>> fixed issues pointed by thierry. >>>> Add PWM compatible string for Hi3519V100. >>>> Implement .apply() function which support atomic, instead of .enable()/.disable()/.config(). >>>> v2: >>>> The number of PWMs is change to be probeable based on the compatible string. >>>> >>>> .../devicetree/bindings/pwm/pwm-hibvt.txt | 23 ++ >>>> drivers/pwm/Kconfig | 9 + >>>> drivers/pwm/Makefile | 1 + >>>> drivers/pwm/pwm-hibvt.c | 270 +++++++++++++++++++++ >>>> 4 files changed, 303 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-hibvt.txt >>>> create mode 100644 drivers/pwm/pwm-hibvt.c >>>> >>>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-hibvt.txt b/Documentation/devicetree/bindings/pwm/pwm-hibvt.txt >>>> new file mode 100644 >>>> index 0000000..609284f >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/pwm/pwm-hibvt.txt >>>> @@ -0,0 +1,23 @@ >>>> +Hisilicon PWM controller >>>> + >>>> +Required properties: >>>> +-compatible: should contain one SoC specific compatible string and one generic compatible >>>> +string "hisilicon, hibvt-pwm". The SoC specific strings supported including: >>> >>> Extra space ^ >>> >>> With that, for the binding: >>> >>> Acked-by: Rob Herring <robh@xxxxxxxxxx> >> >> I'm wondering what the use is of the generic compatible string? Not only >> is the driver not listing it, nor is it in any way useful since there is >> nothing the driver could do based on the generic compatible string (each >> version of the IP currently supported has a different number of PWM >> channels). > > Actually it's worse. The driver does have an entry for the generic > compatible string but it doesn't assign .data in that case (which is > really what I was saying above). Now according to the bindings it is > disallowed to have only the generic compatible string, which is good > because doing so would crash the driver on probe. > > So, if it's not allowed to have only the generic compatible string, > why even have one at all? We always have an SoC-specific compatible > string as well, so the generic one is completely redundant. > > Thierry > Hi, Thierry, are there some other issue about driver except the generic compatible string? awaiting your reply. thanks. Jian Yuan. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
