linux-kernel - Re: [PATCH v1 2/5] pardata: new bus for parallel data access

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:   Wed, 8 Aug 2018 18:22:54 +0200
From:   Noralf Trønnes <noralf@...nnes.org>
To:     Sam Ravnborg <sam@...nborg.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v1 2/5] pardata: new bus for parallel data access


Den 08.08.2018 10.24, skrev Sam Ravnborg:
> Hi Noralf.
>
> On Tue, Aug 07, 2018 at 06:40:29PM +0200, Noralf Trønnes wrote:
>> Hi Sam,
>>
>> Den 02.08.2018 21.45, skrev Sam Ravnborg:
>>> The pardata supports implement a simple bus for devices
>>> that are connected using a parallel bus driven by GPIOs.
>>> The is often used in combination with simple displays
>>> that is often seen in older embedded designs.
>>> There is a demand for this support also in the linux
>>> kernel for HW designs that uses these kind of displays.
>>>
>>> The pardata bus uses a platfrom_driver that when probed
>>> creates devices for all child nodes in the DT,
>>> which are then supposed to be handled by pardata_drivers.
>>>
>>> Signed-off-by: Sam Ravnborg <sam@...nborg.org>
>>> ---
>>  From a quick look at this I have these comments:
>> 1. There can only be one implementation of this bus, the gpio one.
>>     There are SOC's with parallel bus hardware so you need to allow for
>>     more implementations.
>> 2. The client shouldn't do the bus signaling. This should be hidden
>>     behind read and write functions in pardata.
>> 3. I would also suggest you add an address bus instead of the RS pin
>> 4. I don't think reset belongs in the bus. It's a device thing.
> Excellent feedback - I will address this in v2.
>
>> 5. You can use gpiod_set_array_value() in the gpio implementation.
>>     Some gpio drivers can set all gpios at once.
> Yes, it is already used. But only for DB0 to DB7 as there
> are timing constraings for the others.
>
>> I made an attempt at implementing a bus like this a while back:
>> https://github.com/notro/fbdbi/tree/master/i80
> Thanks, very helpfull.
>
> v2 will take a while as I plan to have something that actually works
> before posting next version.
>
> One open question. Miguel Ojeda mentioned that there is already a
> limited fbdev driver made on top of parport (part of auxdisplay).
> Is it the correct design to come up with a new bus or should this
> try to build on top of parport?
>
> I did not check in details if using parport is possible, but from a
> quick look it is doable.
> But then we use parport for something that it originally was not
> designed for and we drag with us a lot of extra functionality.
> So I like the slimmer pardata bus.

I couldn't find a parport gpio driver, so you would have to write one by
the looks of it.

I think it would be best to add a new bus type, but that's just my opinion.
I suggest you study the other bus types (spi, i2c, spmi) to get an
understanding of how it's done.

Noralf.