From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp6-v.fe.bosch.de ([139.15.237.11]:40527 "EHLO smtp6-v.fe.bosch.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753621AbcFAHie (ORCPT ); Wed, 1 Jun 2016 03:38:34 -0400 Subject: Re: [PATCH 0/4] ARM: Renesas: R-Car3: Add product register support To: Geert Uytterhoeven References: <1464166708-18320-1-git-send-email-dirk.behme@de.bosch.com> <426527f7-d1fe-8574-7f25-66298bb55eab@de.bosch.com> CC: Magnus Damm , Geert Uytterhoeven , Simon Horman , , Koji Matsuoka From: Dirk Behme Message-ID: <653cbab7-e7d7-476f-62b1-e9479827cfbb@de.bosch.com> Date: Wed, 1 Jun 2016 09:38:30 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Geert, On 01.06.2016 09:27, Geert Uytterhoeven wrote: > Hi Dirk, > > On Wed, Jun 1, 2016 at 9:19 AM, Dirk Behme wrote: >> On 01.06.2016 07:19, Magnus Damm wrote: >>> On Fri, May 27, 2016 at 4:56 PM, Dirk Behme >>> 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