linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Mylene Josserand <mylene.josserand@collabora.com>
To: "Geert Uytterhoeven" <geert@linux-m68k.org>,
	"Heiko Stübner" <heiko@sntech.de>
Cc: Stephen Boyd <sboyd@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Russell King <linux@armlinux.org.uk>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	Collabora Kernel ML <kernel@collabora.com>,
	linux-clk <linux-clk@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/2] ARM: Rockchip: Handle rk3288/rk3288w revision
Date: Thu, 26 Mar 2020 14:50:22 +0100	[thread overview]
Message-ID: <5802ec08-5e6a-8547-ee8e-dde630791235@collabora.com> (raw)
In-Reply-To: <CAMuHMdXJQqaCcMko9GUAeUiYQzmy3vnX42yVQNPzhj5ijtFuYA@mail.gmail.com>

Hello everyone,

Thank you for all these feedback and discussion about my patches.

On 3/6/20 11:45 AM, Geert Uytterhoeven wrote:
> Hi Heiko,
> 
> On Wed, Mar 4, 2020 at 12:00 PM Heiko Stübner <heiko@sntech.de> wrote:
>> Am Montag, 2. März 2020, 16:57:02 CET schrieb Mylène Josserand:
>>> Determine which revision of rk3288 by checking the HDMI version.
>>> According to the Rockchip BSP kernel, on rk3288w, the HDMI
>>> revision equals 0x1A which is not the case for the rk3288 [1].
>>>
>>> As these SOC have some differences, the new function
>>> 'soc_is_rk3288w' will help us to know on which revision
>>> we are.
>>
>> what happened to just having a different compatible in the dts?
>> Aka doing a
>>
>> rk3288w.dtsi with
>>
>> #include "rk3288.dtsi"
>>
>> &cru {
>>          compatible = "rockchip,rk3288w-cru";
>> }
>>
>> I somehow don't expect boards to just switch between soc variants
>> on the fly.
>>
>> Also, doing things in mach-rockchip is not very future-proof:
>>
>> (1) having random soc-specific APIs spanning the kernel feels wrong,
>>      especially as at some point it might not be contained to our own special
>>      drivers like the cru. I cannot really see people being enthusiastic if
>>      something like this would be needed in say the core Analogix-DP bridge ;-)
> 
> Indeed.  You're better of registering an soc_device_attribute using
> soc_device_register(), after which any driver can use soc_device_match()
> to differentiate based on the SoC revision.

Thank you for this suggestion. The issue is that clocks are registered 
at an early stage of the boot so using initcalls is too late for the 
clock differentiation :(

> 
>> (2) I guess the rk3288w will not be the last soc doing this and on arm64 you
>>      can't do it that way, as there is no mach-rockchip there
> 
> There's drivers/soc/rockchip ;-)
> 
>> So my personal preference would really would be just a specific compatible
>> for affected ip blocks.
> 
> Doing that only for affected IP blocks may miss integration differences.
> Of course, you can always resort to soc_device_match() to handle those...
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

It would be a great idea indeed if there was not this clock registration 
too soon for initcalls.

I am currently running out of ideas to propose a better solution than 
this V1. Is anyone has any recommendation to handle that? I would really 
appreciate :)

Thank you in advance!
Best regards,
Mylène

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-03-26 13:50 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-02 15:57 [PATCH 0/2] ARM: Add Rockchip rk3288w support Mylène Josserand
2020-03-02 15:57 ` [PATCH 1/2] ARM: Rockchip: Handle rk3288/rk3288w revision Mylène Josserand
2020-03-04 10:59   ` Heiko Stübner
2020-03-05  0:03     ` Ezequiel Garcia
2020-03-05  0:51       ` Heiko Stübner
2020-03-05 11:32         ` Ezequiel Garcia
2020-03-05 11:35           ` Ezequiel Garcia
2020-03-06  2:44             ` Kever Yang
2020-03-06 10:30               ` Ezequiel Garcia
2020-03-06 10:45     ` Geert Uytterhoeven
2020-03-26 13:50       ` Mylene Josserand [this message]
2020-03-26 15:31         ` Geert Uytterhoeven
2020-03-27  6:20           ` Mylene Josserand
2020-03-02 15:57 ` [PATCH 2/2] clk: rockchip: rk3288: Handle clock tree for rk3288w Mylène Josserand
2020-03-04  5:34   ` kbuild test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5802ec08-5e6a-8547-ee8e-dde630791235@collabora.com \
    --to=mylene.josserand@collabora.com \
    --cc=geert@linux-m68k.org \
    --cc=heiko@sntech.de \
    --cc=kernel@collabora.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux@armlinux.org.uk \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).