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] [day] [month] [year] [list]
Date:	Tue, 25 Feb 2014 10:43:41 +0900
From:	Magnus Damm <magnus.damm@...il.com>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:	Mark Brown <broonie@...nel.org>,
	Takashi Yoshii <takasi-y@....dti.ne.jp>,
	linux-spi <linux-spi@...r.kernel.org>,
	Linux-sh list <linux-sh@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Geert Uytterhoeven <geert+renesas@...ux-m68k.org>
Subject: Re: [PATCH 08/10] spi: sh-msiof: Move clock management to (un)prepare_message()

Hi Geert,

On Mon, Feb 24, 2014 at 6:48 PM, Geert Uytterhoeven
<geert@...ux-m68k.org> wrote:
> Hi Mark, Magnus,
>
> On Sat, Feb 22, 2014 at 4:27 AM, Mark Brown <broonie@...nel.org> wrote:
>> On Thu, Feb 20, 2014 at 03:43:07PM +0100, Geert Uytterhoeven wrote:
>>
>>> +     if (!test_and_set_bit(0, &p->flags)) {
>>> +             pm_runtime_get_sync(&p->pdev->dev);
>>> +             clk_enable(p->clk);
>>> +     }
>>
>> That test_and_set_bit() is a bit odd - what's going on there, perhaps a
>> comment is in order?  I'd also like to see return value checks, though
>
> My first guess was to support multiple CS, but you can't have multiple
> active SPI slaves at the same time.
>
> Perhaps it's because the bitbang core may call the .chipselect() callback
> multiple times with is_on == BITBANG_CS_ACTIVE, and obviously the
> clock should be enabled/disabled only once?
> The current code doesn't seem to do that, but perhaps it was different when
> the sh-msiof driver was written?
>
> Ah, there's also the initial .chipselect(..., BITBANG_CS_INACTIVE) call
> in spi_bitbang_setup(), which should not disable the clock.
> But it should still call sh_msiof_spi_set_pin_regs() and set the optional
> GPIO CS. Which is no longer done after my series. I'll fix that.
>
> Magnus, do you remember the rationale for the test_and_set_bit()?

Sorry, I don't remember. Perhaps it was related to the CS bitbang handling.

> Anyway, it seems safe to remove it, as .(un)prepare_message() is
> guaranteed to be called in matching pairs.

Yes, I agree!

Thanks,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists