From: Thierry Reding <thierry.reding@gmail.com>
To: Jian Yuan <yuanjian12@hisilicon.com>
Cc: robh+dt@kernel.org, mark.rutland@arm.com,
linux-pwm@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, xuejiancheng@hisilicon.com,
kevin.lixu@hisilicon.com, jalen.hsu@hisilicon.com
Subject: Re: [PATCH v2] pwm: add pwm driver for HiSilicon BVT SOCs
Date: Mon, 5 Sep 2016 12:01:15 +0200 [thread overview]
Message-ID: <20160905100115.GC31424@ulmo.ba.sec> (raw)
In-Reply-To: <09f4946b-0e67-d2f7-cb43-03dead1ec4a9@hisilicon.com>
[-- Attachment #1: Type: text/plain, Size: 2965 bytes --]
On Thu, Aug 25, 2016 at 05:15:36PM +0800, Jian Yuan wrote:
>
>
> On 2016/8/24 21:05, Thierry Reding wrote:
> > On Mon, Aug 22, 2016 at 03:50:13PM +0800, Jian Yuan wrote:
> >> From: yuanjian <yuanjian12@hisilicon.com>
> >>
> >> Add pwm driver for HiSilicon BVT SOCs
> >
> > pwm -> PWM, please. It'd be good to have more information here about
> > what the hardware can do, where to find it, etc.
> >
> Not sure what you mean? Should I describe what the PWM or the BVT SoCs can do?
Some of the things you could mention here are how many channels the PWM
controller supports, if there any noteworthy limitations or extra
features. Looking at the driver the controller is pretty standard, but
you could mention that it supports signal polarity (not all controllers
do) and that it can be found on two SoCs and name them. Sometimes it's
also useful to give some information about typical use-cases (perhaps
one of the PWMs is used to control backlight of a panel with some widely
available development board?).
> >> Reviewed-by: Jiancheng Xue <xuejiancheng@hisilicon.com>
> >> Signed-off-by: Jian Yuan <yuanjian12@hisilicon.com>
> >> ---
> >> Change Log:
> >> v2:
> >> The number of PWMs is change to be probeable based on the compatible string.
> >>
> >> .../devicetree/bindings/pwm/pwm-hibvt.txt | 18 ++
> >> drivers/pwm/Kconfig | 10 +
> >> drivers/pwm/Makefile | 1 +
> >> drivers/pwm/pwm-hibvt.c | 274 +++++++++++++++++++++
> >> 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..1274119
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/pwm/pwm-hibvt.txt
> >> @@ -0,0 +1,18 @@
> >> +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:
> >
> > Why the generic compatible string? You've already shown in the driver
> > that the two versions you support aren't compatible.
> >
> The generic compatible string should be contained in every devicetree
> bindings that BVT Socs support.
> But I'll add another specific compatible string to distinguish it from
> "hisilicon,hi3516cv300-pwm".
There's no use in specifying a generic compatible string if a driver
can't do anything useful with it. If you've only got hisilicon,hibvt-pwm
in the device tree, a driver wouldn't know the target SoC and hence has
no knowledge about the number of channels. Therefore the generic
compatible string is useless.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
prev parent reply other threads:[~2016-09-05 10:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-22 7:50 [PATCH v2] pwm: add pwm driver for HiSilicon BVT SOCs Jian Yuan
2016-08-23 18:08 ` Rob Herring
2016-08-24 13:05 ` Thierry Reding
2016-08-25 9:15 ` Jian Yuan
2016-09-05 10:01 ` Thierry Reding [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160905100115.GC31424@ulmo.ba.sec \
--to=thierry.reding@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=jalen.hsu@hisilicon.com \
--cc=kevin.lixu@hisilicon.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=robh+dt@kernel.org \
--cc=xuejiancheng@hisilicon.com \
--cc=yuanjian12@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).