Re: [PATCH 0/4] ARM: Renesas: R-Car3: Add product register support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Magnus,

On 01.06.2016 07:19, Magnus Damm wrote:
Hi Dirk,

On Fri, May 27, 2016 at 4:56 PM, Dirk Behme <dirk.behme@xxxxxxxxxxxx> wrote:
On 27.05.2016 05:13, Magnus Damm wrote:
I don't think anyone disagrees that it makes sense to be able to
determine ES version during runtime. The questions in my mind are how
to do it



I've made a proposal ;) And I'm happy to discuss technically about it.

Thanks! It would be interesting to know the reason behind your
interest in these things.


Regarding the general removal of hard coded registers:

This breaks virtualization. If you use a virtualization solution, e.g. Xen, which does the memory access rights based on the device tree, each peripheral memory access done by the Linux kernel is trapped by the hypervisor, then. Because it doesn't know anything about the memory range accessed.


Regarding the PRODUCTREG specifically, we already see that it's used for engineering sample (ES) detection in some drivers. So not having a (device tree) based solution for this, does block all driver development (upstreaming) where some ES detection might be needed.


and the urgency.



Regarding the urgency: Someone has accepted the hard coded PRODUCT register
(and MODEMR) being in renesas-drivers, now:

https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/tree/drivers/gpu/drm/rcar-du/rcar_du_crtc.c?h=topic/gen3-latest#n177

This does really hurts us.

Therefore, I try to get this removed as soon as possible to hopefully get a
Renesas BSP without this, the next time.

If anybody finds an other way to remove that, that would be fine, too.

Please share the underlying issue so we can fix that.


See above.


So far we have decided to use the compatible string since it is a
common driver model abstraction already used by device drivers and
hardware description in DT. Using DT compat string matching we can
have logic in the drivers to handle ES differences if needed. As for
how to enable the workaround, my opinion is that the missing piece
consists of ES workaround code that appends ES suffix information to
the DT compat strings automatically during boot.

Technically this sounds slightly too complicated to me. As I have to
advocate my proposal (inspired from an other mainline SoC family) I'd think
that my proposal is easier.

It may indeed be easier in the short term, but I feel we need to
consider how to support a wide range of SoCs in a consistent way.

I suppose the workaround is not yet implemented because no one has
deemed this topic as particularly urgent. Until it becomes urgent or a
new ES version appears the affected users can simply adjust the DT
binding themselves. This is what happened to the "sata-r8a7790-es1"
case above.

Can you see any technical reason why using DT compat strings wouldn't
solve your case?


It most probably will work technically, but I see a lot overhead regarding boot time and maintenance in the proposed DT compat strings solution.

That solution would mean that for each ESx "workaround" we have to do in a driver

a) the generic detection code has to be extended to create the driver specific *and* ESx specific compatible node

b) the driver has to be extended to contain the new runtime compatible node

c) the driver has to be extended to handle the ESx specific "workaround"

d) the device tree documentation has to be extended for the new compatible

These are the implementation / maintenance steps. Additionally, regarding the runtime (boot time does matter!) this does mean that at kernel's boot time the ESx information from the hardware register is "translated" into a device tree compatible, this is added to the device tree at runtime, the device driver has to check the device tree and then apply the specific ESx handling.

With my solution, from above list only (c) has to be done. That means that you don't have to touch any generic code. And the driver has just to call one function.

If you like, compare the mainline i.MX6 way of doing this.


Best regards

Dirk




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux