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@xxxxxxxxxx/T/#m6c1ae9b74fceafc4042f7598b1bc594e68e5ec76
>
> Thanks.
> Binbin
>
>
> On Mon, May 29, 2023 at 2:24 PM Conor Dooley <conor@xxxxxxxxxx> wrote:
> >
> >
> >
> > On 29 May 2023 03:59:57 IST, Keguang Zhang <keguang.zhang@xxxxxxxxx> wrote:
> > >On Sun, May 28, 2023 at 6:22 AM Conor Dooley <conor@xxxxxxxxxx> wrote:
> > >>
> > >> On Sat, May 27, 2023 at 10:59:48PM +0100, Jiaxun Yang wrote:
> > >> > > 2023年5月27日 17:23，Conor Dooley <conor@xxxxxxxxxx> 写道：
> > >> > > 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
