On 14/07/2018 15:50:08+0200, Paul Cercueil wrote:

quoted

quoted

 > This would avoid that change (and the test would preferably be
 > (rtc->type == ID_JZ4780))

 That branch should be taken if the SoC is JZ4760, JZ4770 or JZ4780.
 It should not be taken if the SoC is JZ4740 or JZ4725B.

Sure but you can achieve that with only 2 ids...

quoted

 > >   		ret = jz4780_rtc_enable_write(rtc);
 > >   	if (ret == 0)
 > >   		ret = jz4740_rtc_wait_write_ready(rtc);
 > >  @@ -300,6 +303,9 @@ static void jz4740_rtc_power_off(void)
 > >
 > >   static const struct of_device_id jz4740_rtc_of_match[] = {
 > >   	{ .compatible = "ingenic,jz4740-rtc", .data = (void
*)ID_JZ4740 },
 > >  +	{ .compatible = "ingenic,jz4725b-rtc", .data = (void
*)ID_JZ4725B
 > > },
 > >  +	{ .compatible = "ingenic,jz4760-rtc", .data = (void
*)ID_JZ4760 },
 > >  +	{ .compatible = "ingenic,jz4770-rtc", .data = (void
*)ID_JZ4770 },

By doing the correct mapping here e.g:

{ .compatible = "ingenic,jz4725b-rtc", .data = (void *)ID_JZ4740 },

Not very pretty and future-proof if you ask me...
But you're the boss...

I think it makes the code simpler to follow . Regarding future
proofness, you will have to add code and probably use a switch case at
the time you need to handle an RTC differently. At that time, the >=
test trick will not work anymore anyway.


-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com