All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Cc: linux-pwm@vger.kernel.org, punit1.agrawal@toshiba.co.jp,
	devicetree@vger.kernel.org, yuji2.ishikawa@toshiba.co.jp,
	linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	kernel@pengutronix.de, Lee Jones <lee.jones@linaro.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
Date: Thu, 8 Apr 2021 09:59:20 +0200	[thread overview]
Message-ID: <20210408075920.rfrcqxec2yvepj3y@pengutronix.de> (raw)
In-Reply-To: <20210407231548.4paov2fb33cpxxui@toshiba.co.jp>

[-- Attachment #1: Type: text/plain, Size: 1483 bytes --]

Hello Nobuhiro,

On Thu, Apr 08, 2021 at 08:15:48AM +0900, Nobuhiro Iwamatsu wrote:
> > > +	/*
> > > +	 * pwmc is a 2-bit divider for the input clock running at 1 MHz.
> > > +	 * When the settings of the PWM are modified, the new values are shadowed in hardware until
> > > +	 * the period register (PCSR) is written and the currently running period is completed. This
> > > +	 * way the hardware switches atomically from the old setting to the new.
> > > +	 * Also, disabling the hardware completes the currently running period and keeps the output
> > > +	 * at low level at all times.
> > 
> > Did you just copy my optimal description or is your hardware really that
> > nice?
> 
> Yes, this hardware works as you wrote.
> And I added about the state if the sinnal when this hardware disabled.
> 
> > 
> > Do you know scripts/checkpatch.pl? I bet it will tell you to limit your
> > lines to approx. 80 chars where sensible.
> 
> Yes, I know. I ran scripts/checkpatch.pl before send patch.
> I understand that the number of characters per line has been changed to
> 100 characters. Does the pwm driver recommend 80 characters?

For free-text comments I'd still recommend 80, yes. For code lines I'd
be indeed more lax, as a line break in function calls reduces readability.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Cc: linux-pwm@vger.kernel.org, punit1.agrawal@toshiba.co.jp,
	devicetree@vger.kernel.org, yuji2.ishikawa@toshiba.co.jp,
	linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	kernel@pengutronix.de, Lee Jones <lee.jones@linaro.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
Date: Thu, 8 Apr 2021 09:59:20 +0200	[thread overview]
Message-ID: <20210408075920.rfrcqxec2yvepj3y@pengutronix.de> (raw)
In-Reply-To: <20210407231548.4paov2fb33cpxxui@toshiba.co.jp>


[-- Attachment #1.1: Type: text/plain, Size: 1483 bytes --]

Hello Nobuhiro,

On Thu, Apr 08, 2021 at 08:15:48AM +0900, Nobuhiro Iwamatsu wrote:
> > > +	/*
> > > +	 * pwmc is a 2-bit divider for the input clock running at 1 MHz.
> > > +	 * When the settings of the PWM are modified, the new values are shadowed in hardware until
> > > +	 * the period register (PCSR) is written and the currently running period is completed. This
> > > +	 * way the hardware switches atomically from the old setting to the new.
> > > +	 * Also, disabling the hardware completes the currently running period and keeps the output
> > > +	 * at low level at all times.
> > 
> > Did you just copy my optimal description or is your hardware really that
> > nice?
> 
> Yes, this hardware works as you wrote.
> And I added about the state if the sinnal when this hardware disabled.
> 
> > 
> > Do you know scripts/checkpatch.pl? I bet it will tell you to limit your
> > lines to approx. 80 chars where sensible.
> 
> Yes, I know. I ran scripts/checkpatch.pl before send patch.
> I understand that the number of characters per line has been changed to
> 100 characters. Does the pwm driver recommend 80 characters?

For free-text comments I'd still recommend 80, yes. For code lines I'd
be indeed more lax, as a line break in function calls reduces readability.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-04-08  7:59 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-12 13:19 [PATCH v2 0/2] pwm: visconti: Add Toshiba Visconti SoC PWM support Nobuhiro Iwamatsu
2021-02-12 13:19 ` Nobuhiro Iwamatsu
2021-02-12 13:19 ` [PATCH v2 1/2] dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller Nobuhiro Iwamatsu
2021-02-12 13:19   ` Nobuhiro Iwamatsu
2021-03-05 20:22   ` Rob Herring
2021-03-05 20:22     ` Rob Herring
2021-04-09  7:44     ` Nobuhiro Iwamatsu
2021-04-09  7:44       ` Nobuhiro Iwamatsu
2021-02-12 13:19 ` [PATCH v2 2/2] pwm: visconti: Add Toshiba Visconti SoC PWM support Nobuhiro Iwamatsu
2021-02-12 13:19   ` Nobuhiro Iwamatsu
2021-02-12 16:41   ` Uwe Kleine-König
2021-02-12 16:41     ` Uwe Kleine-König
2021-04-07 23:15     ` Nobuhiro Iwamatsu
2021-04-07 23:15       ` Nobuhiro Iwamatsu
2021-04-08  7:59       ` Uwe Kleine-König [this message]
2021-04-08  7:59         ` Uwe Kleine-König
2021-04-08 12:22         ` Thierry Reding
2021-04-08 12:22           ` Thierry Reding

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=20210408075920.rfrcqxec2yvepj3y@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=devicetree@vger.kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=nobuhiro1.iwamatsu@toshiba.co.jp \
    --cc=punit1.agrawal@toshiba.co.jp \
    --cc=robh+dt@kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=yuji2.ishikawa@toshiba.co.jp \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.