linux-kernel - Re: [PATCH 1/2] spi: Add Renesas R-Car RPC SPI controller driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Nov 2018 14:32:09 +0100
From:   Boris Brezillon <boris.brezillon@...tlin.com>
To:     Marek Vasut <marek.vasut@...il.com>
Cc:     masonccyang@...c.com.tw, broonie@...nel.org,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Simon Horman <horms@...ge.net.au>, juliensu@...c.com.tw,
        linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
        linux-spi@...r.kernel.org, tpiepho@...inj.com,
        zhengxunli@...c.com.tw
Subject: Re: [PATCH 1/2] spi: Add Renesas R-Car RPC SPI controller driver

On Tue, 20 Nov 2018 14:09:05 +0100
Marek Vasut <marek.vasut@...il.com> wrote:

> On 11/20/2018 08:23 AM, masonccyang@...c.com.tw wrote:
> > Hi Marek,  
> 
> Hi,
> 
> >> Marek Vasut <marek.vasut@...il.com>
> >> 2018/11/19 下午 10:12
> >>
> >> To
> >>  
> >> > +
> >> > +static int rpc_spi_set_freq(struct rpc_spi *rpc, unsigned long freq)
> >> > +{
> >> > +   int ret;
> >> > +
> >> > +   if (rpc->cur_speed_hz == freq)
> >> > +      return 0;
> >> > +
> >> > +   clk_disable_unprepare(rpc->clk_rpc);
> >> > +   ret = clk_set_rate(rpc->clk_rpc, freq);
> >> > +   if (ret)
> >> > +      return ret;
> >> > +
> >> > +   ret = clk_prepare_enable(rpc->clk_rpc);
> >> > +   if (ret)
> >> > +      return ret;  
> >>
> >> Is this clock disable/update/enable really needed ? I'd think that
> >> clk_set_rate() would handle the rate update correctly.  
> > 
> > This is for run time PM mechanism in spi-mem layer and __spi_sync(),
> > you may refer to another patch [1].
> > 
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=for-4.21&id=b942d80b0a394e8ea18fce3b032b4700439e8ca3  
> 
> I think Geert commented on the clock topic, so let's move it there.
> Disabling and enabling clock to change their rate looks real odd to me.

Look at the CLK_SET_RATE_GATE definition and its users and you'll see
it's not unusual to have such constraints on clks. Maybe your HW does
not have such constraints, but it's not particularly odd to do that
(though it could probably be automated by the clk framework somehow).