All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Richardson <jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
To: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Florian Fainelli
	<f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Dmitry Torokhov <dtor-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Anatol Pomazau <anatol-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Scott Branden <sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Ian Campbell
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	<linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	bcm-kernel-feedback-list
	<bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Rafal Milecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Brian Norris
	<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH 4/4] spi: bcm-mspi: Add support to set serial baud clock rate
Date: Mon, 6 Apr 2015 11:54:10 -0700	[thread overview]
Message-ID: <5522D652.4020205@broadcom.com> (raw)
In-Reply-To: <20150406094636.GC6023-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>

On 15-04-06 02:46 AM, Mark Brown wrote:
> On Sat, Apr 04, 2015 at 12:12:59PM -0700, Florian Fainelli wrote:
>> Le 02/04/2015 12:23, Jonathan Richardson a écrit :
> 
>>> +		/* Calculate SPBR if clock-frequency provided. */
>>> +		if (of_property_read_u32(dev->of_node, "clock-frequency",
>>> +			&desired_rate) >= 0) {
>>> +			u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate);
> 
>> Usually, specifying a "clock-frequency" property is done when there is
>> no clock provider available, yet we take this code path only if we could
>> find a "mspi_clk" which sounds a litle weird.
> 
>> Once there is a proper "mspi_clk" clock, I would make it mandatory for
>> the clock provider to be able to provide the rate as well?
> 
> As far as I can tell it's already mandatory if the property is present -
> it's taking the clock presented to the block and then using an internal
> divider to bring that down to the desired rate.
> 
> We are missing error handling though.
> 

Thanks for the review. Yes that's correct. I also tried to make it
backwards compatible with the current version of the driver where you
can ignore configuring the frequency. The result being ref clock
frequency / 2 * 255. If you provide the clock then it will
enable/prepare it. If you also provide clock-frequency then it will
configure the SPBR. If you don't provide anything then it works as
before - it assumes the clock is already enabled and uses the h/w
default SPBR.

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Richardson <jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
To: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Florian Fainelli
	<f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Dmitry Torokhov <dtor-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Anatol Pomazau <anatol-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Scott Branden <sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Ian Campbell
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	bcm-kernel-feedback-list
	<bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Rafal Milecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Brian Norris
	<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH 4/4] spi: bcm-mspi: Add support to set serial baud clock rate
Date: Mon, 6 Apr 2015 11:54:10 -0700	[thread overview]
Message-ID: <5522D652.4020205@broadcom.com> (raw)
In-Reply-To: <20150406094636.GC6023-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>

On 15-04-06 02:46 AM, Mark Brown wrote:
> On Sat, Apr 04, 2015 at 12:12:59PM -0700, Florian Fainelli wrote:
>> Le 02/04/2015 12:23, Jonathan Richardson a écrit :
> 
>>> +		/* Calculate SPBR if clock-frequency provided. */
>>> +		if (of_property_read_u32(dev->of_node, "clock-frequency",
>>> +			&desired_rate) >= 0) {
>>> +			u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate);
> 
>> Usually, specifying a "clock-frequency" property is done when there is
>> no clock provider available, yet we take this code path only if we could
>> find a "mspi_clk" which sounds a litle weird.
> 
>> Once there is a proper "mspi_clk" clock, I would make it mandatory for
>> the clock provider to be able to provide the rate as well?
> 
> As far as I can tell it's already mandatory if the property is present -
> it's taking the clock presented to the block and then using an internal
> divider to bring that down to the desired rate.
> 
> We are missing error handling though.
> 

Thanks for the review. Yes that's correct. I also tried to make it
backwards compatible with the current version of the driver where you
can ignore configuring the frequency. The result being ref clock
frequency / 2 * 255. If you provide the clock then it will
enable/prepare it. If you also provide clock-frequency then it will
configure the SPBR. If you don't provide anything then it works as
before - it assumes the clock is already enabled and uses the h/w
default SPBR.

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Richardson <jonathar@broadcom.com>
To: Mark Brown <broonie@kernel.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	Dmitry Torokhov <dtor@google.com>,
	Anatol Pomazau <anatol@google.com>,
	Scott Branden <sbranden@broadcom.com>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>, <linux-kernel@vger.kernel.org>,
	<linux-spi@vger.kernel.org>,
	bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
	<devicetree@vger.kernel.org>, Rafal Milecki <zajec5@gmail.com>,
	Brian Norris <computersforpeace@gmail.com>,
	Kevin Cernekee <cernekee@gmail.com>
Subject: Re: [PATCH 4/4] spi: bcm-mspi: Add support to set serial baud clock rate
Date: Mon, 6 Apr 2015 11:54:10 -0700	[thread overview]
Message-ID: <5522D652.4020205@broadcom.com> (raw)
In-Reply-To: <20150406094636.GC6023@sirena.org.uk>

On 15-04-06 02:46 AM, Mark Brown wrote:
> On Sat, Apr 04, 2015 at 12:12:59PM -0700, Florian Fainelli wrote:
>> Le 02/04/2015 12:23, Jonathan Richardson a écrit :
> 
>>> +		/* Calculate SPBR if clock-frequency provided. */
>>> +		if (of_property_read_u32(dev->of_node, "clock-frequency",
>>> +			&desired_rate) >= 0) {
>>> +			u32 spbr = clk_get_rate(data->clk) / (2 * desired_rate);
> 
>> Usually, specifying a "clock-frequency" property is done when there is
>> no clock provider available, yet we take this code path only if we could
>> find a "mspi_clk" which sounds a litle weird.
> 
>> Once there is a proper "mspi_clk" clock, I would make it mandatory for
>> the clock provider to be able to provide the rate as well?
> 
> As far as I can tell it's already mandatory if the property is present -
> it's taking the clock presented to the block and then using an internal
> divider to bring that down to the desired rate.
> 
> We are missing error handling though.
> 

Thanks for the review. Yes that's correct. I also tried to make it
backwards compatible with the current version of the driver where you
can ignore configuring the frequency. The result being ref clock
frequency / 2 * 255. If you provide the clock then it will
enable/prepare it. If you also provide clock-frequency then it will
configure the SPBR. If you don't provide anything then it works as
before - it assumes the clock is already enabled and uses the h/w
default SPBR.


  parent reply	other threads:[~2015-04-06 18:54 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-02 19:23 [PATCH 0/4] Add MSPI support for Cygnus Jonathan Richardson
2015-04-02 19:23 ` Jonathan Richardson
2015-04-02 19:23 ` Jonathan Richardson
     [not found] ` <1428002603-21892-1-git-send-email-jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-04-02 19:23   ` [PATCH 1/4] ARM: dts: Add binding for Broadcom MSPI driver Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
2015-04-04 19:17     ` Florian Fainelli
2015-04-06 18:45       ` Jonathan Richardson
2015-04-06 18:45         ` Jonathan Richardson
2015-04-02 19:23   ` [PATCH 2/4] spi: bcm53xx: Refactor to make driver nonspecific to 53xx SoCs Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
2015-04-03 13:35     ` Andy Shevchenko
     [not found]       ` <CAHp75Vf0JrB5pqNEouLzyOf5mmi05Ut_cybd37suTrLbJOFHKA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-04-06 10:18         ` Rafał Miłecki
2015-04-06 10:18           ` Rafał Miłecki
2015-04-06 18:58           ` Jonathan Richardson
2015-04-06 18:30       ` Jonathan Richardson
2015-04-06 18:30         ` Jonathan Richardson
     [not found]         ` <5522D0BA.2050805-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-04-07  8:03           ` Andy Shevchenko
2015-04-07  8:03             ` Andy Shevchenko
2015-04-02 19:23   ` [PATCH 3/4] spi: bcm-mspi: Make BCMA optional to support non-BCMA chips Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
     [not found]     ` <1428002603-21892-4-git-send-email-jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-04-03 13:38       ` Andy Shevchenko
2015-04-03 13:38         ` Andy Shevchenko
     [not found]         ` <CAHp75VcMg-8X8DtHbQrmugzWG5VzzHXcpWNiyrTT=qmuCvA95w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-04-03 17:52           ` Florian Fainelli
2015-04-03 17:52             ` Florian Fainelli
     [not found]             ` <551ED345.1000702-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-04-06 10:36               ` Rafał Miłecki
2015-04-06 10:36                 ` Rafał Miłecki
2015-04-06 19:09                 ` Jonathan Richardson
2015-04-06 18:39               ` Jonathan Richardson
2015-04-06 18:39                 ` Jonathan Richardson
2015-04-06 18:39                 ` Jonathan Richardson
2015-04-06 10:26           ` Rafał Miłecki
2015-04-06 10:26             ` Rafał Miłecki
2015-04-06 10:26       ` Rafał Miłecki
2015-04-06 10:26         ` Rafał Miłecki
2015-04-02 19:23   ` [PATCH 4/4] spi: bcm-mspi: Add support to set serial baud clock rate Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
2015-04-02 19:23     ` Jonathan Richardson
     [not found]     ` <1428002603-21892-5-git-send-email-jonathar-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2015-04-04 19:12       ` Florian Fainelli
2015-04-04 19:12         ` Florian Fainelli
     [not found]         ` <552037BB.7050803-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-04-06  9:46           ` Mark Brown
2015-04-06  9:46             ` Mark Brown
     [not found]             ` <20150406094636.GC6023-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-04-06 18:54               ` Jonathan Richardson [this message]
2015-04-06 18:54                 ` Jonathan Richardson
2015-04-06 18:54                 ` Jonathan Richardson

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=5522D652.4020205@broadcom.com \
    --to=jonathar-dy08kvg/lbpwk0htik3j/w@public.gmane.org \
    --cc=anatol-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=dtor-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=sbranden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \
    --cc=zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    /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.