linux-kernel - Re: [PATCH v2 2/2] devicetree: Add devicetree bindings documentation for Cadence SPI

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 for Android: free password hash cracker in your pocket

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

Message-ID: <533E94AB.6060400@monstr.eu>
Date:	Fri, 04 Apr 2014 13:16:59 +0200
From:	Michal Simek <monstr@...str.eu>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
CC:	Harini Katakam <harinikatakamlinux@...il.com>,
	Mark Brown <broonie@...nel.org>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	linux-spi <linux-spi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] devicetree: Add devicetree bindings documentation
 for Cadence SPI

On 04/04/2014 10:09 AM, Geert Uytterhoeven wrote:
> Hi Harini,
> 
> On Fri, Apr 4, 2014 at 5:00 AM, Harini Katakam
> <harinikatakamlinux@...il.com> wrote:
>>>> +             num-cs = /bits/ 16 <4>;
>>>
>>> What's going on with the /bits/ - is this something that's required for
>>> the property?
>>
>> The master->num-chipselect property is 16 bit but writing <4> here directly
>> leads to 0 being read in of_property_read (because it's big endian).
>> Instead using of property read u32 and then copying, we decided to do this.
>> This was discussed on v2 between Michal and Rob:
>>>>>> +               num-chip-select = /bits/ 16 <4>;
>>>>
>>>> I was expecting you will comment this a little bit. :-)
>>>> Because all just reading this num-cs as 32bit and then
>>>> assigning this value to master->num_chipselect which is 16bit.
>>>
>>> Well, everyone else has that problem then. Obviously it takes a bit
>>> more care than just reading into a u32, but that is a kernel problem
>>> and not a problem of the binding.
>> They are not reading it directly with read_u32 but they are using
>> intermediate u32 value which is assigned to u16 which is fine.
>> This pattern is in most drivers(maybe all).
>> The point is if binding should or can't simplify driver code.
>> And from your reaction above I expect that it is up to driver
>> owner and binding doc how you want to do it.
> 
> IMHO this "/bits/ 16" doesn't simplify the binding.
> 
> As "num-cs" is a generic spi subsystem binding, it should not be
> restricted to 16 bits for the sake of a driver. As your hardware can drive 4
> chip selects, you could represent it in 3 bits (don't!).
> 
> Simple integers are 32 bit in DT, so use a temporary.

No problem to keep it there to 32bit range. I really appreciate
that discussion we have about it.

Just a note: If "num-cs" is the part of generic spi subsystem binding
maybe it should be moved directly to spi core as is done for
example for timeout-sec in watchdog.
Also this should be listed in any Documentation/devicetree/bindings/spi/spi.txt
file that this is generic spi binding.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Download attachment "signature.asc" of type "application/pgp-signature" (264 bytes)