linux-kernel - Re: [PATCH 04/11] ARM: shmobile: lager legacy: Add MSIOF support

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]

Message-ID: <CAMuHMdWDtDqmarg-p+LFv4ZtzF7Rv=KzLQA3rnSa_dS2PkVmfA@mail.gmail.com>
Date:	Mon, 24 Feb 2014 09:25:08 +0100
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Magnus Damm <magnus.damm@...il.com>
Cc:	Simon Horman <horms@...ge.net.au>,
	SH-Linux <linux-sh@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Geert Uytterhoeven <geert+renesas@...ux-m68k.org>
Subject: Re: [PATCH 04/11] ARM: shmobile: lager legacy: Add MSIOF support

Hi Magnus,

On Mon, Feb 24, 2014 at 3:09 AM, Magnus Damm <magnus.damm@...il.com> wrote:
> On Fri, Feb 21, 2014 at 1:18 AM, Geert Uytterhoeven
> <geert@...ux-m68k.org> wrote:
>> On Thu, Feb 20, 2014 at 4:48 PM, Magnus Damm <magnus.damm@...il.com> wrote:
>>> Since only MSIOF1 is used on Lager (correct me if i'm wrong), isn't it
>>> best to omit the unused resources from above? In case of DT I think it
>>> makes sense to define all channels in the SoC.dtsi and let the
>>> SoC-board.dts just enable the channels that are used. But in this case
>>> with legacy code  I think we should keep thing simple and small and
>>> just enable the bits that are used on the particular board.
>>>
>>> The same obviously applies to the Koelsch legacy code as well. =)
>>
>> Note that while all resources are present, only MSIOF1 is registered on
>> Lager (MSIOF0 on Koelsch). This is similar to i2c on Koelsch, which also
>> has all resources, but only registers active devices.
>
> Ok, I understand. Thanks for brining this to my attention.
>
> I'd like to avoid having unused resources so I'll cook up a patch to
> rework that myself.
>
>> It's your preference, though, so I can adapt if you want.
>
> Thanks.
>
> Please rework this patch to only register a single MSIOF channel. I
> think it makes sense to only enable hardware that is being used.

Ehrm, I already register a single MSIOF channel only.
Perhaps you mean't "remove the unused resources"?

> Another question: How about "bus_num" and the platform device id
> mapping? I'd like them to be the same if possible, but you are having
> this "(idx+1)" bit in your code which I assume is to add offset for
> the QSPI bus.

"bus_num" is the SPI-specific numbering of SPI masters, which is filled
in by spi-sh-msiof.c based on platform_device.id (i.e. the numeric suffix
of e.g. "spi_r8a7790_msiof.1").
It's used for matching SPI slaves in spi_board_info with SPI masters.
As QSPI ("qspi.0") has SPI bus_num 0, the MSIOF SPI masters use
bus_num 1 to 4, hence the "idx+1", and the platform device names
"spi_r8a7790_msiof.1" to "spi_r8a7790_msiof.4".

(Can't spi-sh-msiof.c use "bus_num = pdev->id + 1", so we can have
 MSIOF0 as "spi_r8a7790_msiof.0"? No, as that would impact numbering
 on all SoCs with MSIOF.)

With DT, all of this doesn't matter, and life is easier, as the SPI slaves
are child nodes of the SPI masters and thus don't need numerical bus
references. So MSIOF0 can be called "msiof0" there.
We still have the offsets in the "spi%u" aliases, though.

> Regarding the PFC configuration, can you please double check that the
> PIN_MAP_MUX_GROUP_DEFAULT() is in sync with the platform device id? Is
> it the "bus_num" or the platform device id that is being used in case
> of SPI?

"bus_num" is SPI-specific. Pinctrl uses the actual device's name:

/**
 * struct pinctrl_map - boards/machines shall provide this map for devices
 * @dev_name: the name of the device using this specific mapping, the name
 *      must be the same as in your struct device*. If this name is set to the
 *      same name as the pin controllers own dev_name(), the map entry will be
 *      hogged by the driver itself upon registration

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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/