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 Geert,

On 01.06.2016 09:27, Geert Uytterhoeven wrote:
Hi Dirk,

On Wed, Jun 1, 2016 at 9:19 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxx> wrote:
On 01.06.2016 07:19, Magnus Damm wrote:
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. For instance, if you are interested in
reducing run time memory usage, or general source code duplication
reduction. Do you have any target platform that you can upstream board
support for?

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.

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?

I'll try to explain my understanding. Please re-ask or correct me if I fail
;)

First of all, I can talk only about R-Car3. There, at the moment, we have
ES1, ES2 and ES3 documented, already. We don't know, yet, if these are
really needed in the code, but it might be.

Taking the example above, we then would have

compatible = "renesas,sata-r8a7790", "sata-r8a7790-es1", "sata-r8a7790-es2",
"sata-r8a7790-es3";

in the device tree. Do we want that? No! Because its no run time detection.

You would only have "renesas,sata-r8a7790" for normal parts.
ES1 needs "renesas,sata-r8a7790-es1".
"esX" compatible values are the exception, not the rule.


Yes.


If we don't want that, and we want runtime detection, I think anywhere in
this thread it was mentioned that the "es1" string is appended to
sata-r8a7790 at runtime? If so, could you point us to the code for that?

The PoC is on my local SSD. Will send out soon.

Then we have anything like

if(PRODUCT_REG & ES1 == ES1)
        append ES1 to the compatible string

in the boot code. This is one additional indirection to my proposal, needing
boot time. In the driver you then add

        {
                .compatible = "renesas,sata-r8a7790-es1",
                .data = (void *)RCAR_R8A7790_ES1_SATA
        },
        {
                .compatible = "renesas,sata-r8a7790-es2",
                .data = (void *)RCAR_R8A7790_ES2_SATA
        },
        {
                .compatible = "renesas,sata-r8a7790-es3",
                .data = (void *)RCAR_R8A7790_ES3_SATA
        },

No, only "renesas,sata-r8a7790" and "renesas,sata-r8a7790-es1", like we
already have.


Sorry, if that was unclear. I took the example and transferred it to R-Car3 where we have ES1 - ES3.

So, taking this example, on R-Car3 we might end up with

	{
		.compatible = "renesas,sata-r8a7795",
		.data = (void *)RCAR_GEN2_SATA
	},
	{
		.compatible = "renesas,sata-r8a7795-es1",
		.data = (void *)RCAR_R8A7795_ES1_SATA
	},
	{
		.compatible = "renesas,sata-r8a7795-es2",
		.data = (void *)RCAR_R8A7795_ES2_SATA
	},
	{
		.compatible = "renesas,sata-r8a7795-es3",
		.data = (void *)RCAR_R8A7795_ES3_SATA
	},

?

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