On Tue, May 30, 2023 at 6:20 AM Alexandre Belloni
<alexandre.belloni@xxxxxxxxxxx> wrote:
>
> 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.
Hi Alexandre:
I am glad to receive your reply.
Yes, we tested and found that ls1c indeed can't trigger alarm
interrupts, but can read and write RTC time normally.
Also, in the latest rtc driver (V4), we measure the alarm function by
the interrupts property.
Therefore, whether the ls1c compatible can be retained?
Thanks.
Binbin
>
> 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
