Re: [PATCH V4 1/5] dt-bindings: rtc: Remove the LS2X from the trivial RTCs

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

Hello,

Honestly, the list of compatibles is fine for me. I wouldn't go for
fallback. The improvement would be to drop "loongson,ls1c-rtc",
and probably "loongson,ls2k0500-rtc" and "loongson,ls2k2000-rtc".

loongson,ls1c-rtc is definitively not needed, the alarm may not be wired
but the registers are there.

For 2k0500 and 2k2000, I don't mind either way.

On 29/05/2023 16:31:42+0800, Binbin Zhou wrote:
> Hi Krzysztof:
> 
> Excuse me.
> We have different opinions on how to better describe rtc-loongson compatible.
> 
> Based on my previous communication with you, I think we should list
> all the Socs in the driver and drop the wildcards.
> This should be clearer and more straightforward:
> 
>         { .compatible = "loongson,ls1b-rtc", .data = &ls1x_rtc_config
> }, //ls1b soc
>         { .compatible = "loongson,ls1c-rtc", .data = &ls1x_rtc_config
> }, //ls1c soc
>         { .compatible = "loongson,ls7a-rtc", .data =
> &generic_rtc_config }, //ls7a bridge chip
>         { .compatible = "loongson,ls2k0500-rtc", .data =
> &generic_rtc_config }, // ls2k0500 soc
>         { .compatible = "loongson,ls2k2000-rtc", .data =
> &generic_rtc_config }, // ls2k2000 soc
>         { .compatible = "loongson,ls2k1000-rtc", .data =
> &ls2k1000_rtc_config }, // ls2k1000 soc
> 
> And Conor thought it should be rendered using a fallback compatible
> form based on ".data".
> 
>         "loongson,ls1b-rtc"
>         "loongson,ls1c-rtc", "loongson,ls1b-rtc"
>         "loongson,ls7a-rtc"
>         "loongson,ls2k0500-rtc", "loongson,ls7a-rtc"
>         "longson,ls2k2000-rtc", "longson,ls7a-rtc"
>         "loonson,ls2k1000-rtc"
> 
>         { .compatible = "loongson,ls1b-rtc", .data = &ls1x_rtc_config }
>         { .compatible = "loongson,ls7a-rtc", .data = &generic_rtc_config }
>         { .compatible = "loongson,ls2k1000-rtc", .data = &ls2k1000_rtc_config }
> 
> In this form,  I think it might not be possible to show very
> graphically which chips are using the driver.
> Also, for example, "ls7a" is a bridge chip, while
> "ls2k2000"/"ls2k0500" are soc chips, and it seems inappropriate to
> integrate them into one item.
> 
> Which one do you think is more suitable for us?
> 
> Here is the link to our discussion:
> 
> https://lore.kernel.org/linux-rtc/E229B204-1B00-4B24-B4BF-15277682FB4B@kernel.org/T/#m6c1ae9b74fceafc4042f7598b1bc594e68e5ec76
> 
> Thanks.
> Binbin
> 
> 
> On Mon, May 29, 2023 at 2:24 PM Conor Dooley <conor@kernel.org> wrote:
> >
> >
> >
> > On 29 May 2023 03:59:57 IST, Keguang Zhang <keguang.zhang@gmail.com> wrote:
> > >On Sun, May 28, 2023 at 6:22 AM Conor Dooley <conor@kernel.org> wrote:
> > >>
> > >> On Sat, May 27, 2023 at 10:59:48PM +0100, Jiaxun Yang wrote:
> > >> > > 2023年5月27日 17:23，Conor Dooley <conor@kernel.org> 写道：
> > >> > > On Sat, May 27, 2023 at 05:13:39PM +0100, Jiaxun Yang wrote:
> > >>
> > >> > >> My recommendation is leaving compatible string as is.
> > >> > >
> > >> > > "as is" meaning "as it is right now in Linus' tree", or "as it is in
> > >> > > this patch"?
> > >> >
> > >> > Ah sorry I meant in this patch.
> > >> >
> > >> > Since there won’t be any new ls1x chip that will boot Linux any time soon (due to
> > >> > Loongson move away from MIPS but LoongArch32 is undefined for now), and
> > >> > rest compatible strings are wide enough to cover their family, I think the present
> > >> > compatible strings in this patch describes hardware best.
> > >>
> > >> I don't see why new bindings being written for old hardware should somehow
> > >> be treated differently than new bindings for new hardware.
> > >
> > >Let me add that ls1b RTC and ls1c RTC are not exactly the same.
> > >The former supports RTC interrupt, while the latter does not.
> > >So my suggestion is to leave the compatible string as it is in this patch.
> >
> > Just as a reminder, there are more than ls1b & c in the patch, lest we forget.
> > Also, fallback compatibles mean a compatible subset, not only that two devices are identical.
> > The interrupt is passed by the interrupts property.
> >

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com