diff for duplicates of <1532355640.2289.0@smtp.crapouillou.net> diff --git a/a/1.txt b/N1/1.txt index fdae8f0..ae390b2 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,30 +1,30 @@ Hi, -Le lun. 23 juil. 2018 =E0 16:18, Rob Herring <robh@kernel.org> a =E9crit : -> On Fri, Jul 20, 2018 at 1:04 PM Paul Cercueil <paul@crapouillou.net>=20 +Le lun. 23 juil. 2018 à 16:18, Rob Herring <robh@kernel.org> a écrit : +> On Fri, Jul 20, 2018 at 1:04 PM Paul Cercueil <paul@crapouillou.net> > wrote: ->>=20 +>> >> Hi, ->>=20 ->> Le ven. 20 juil. 2018 =E0 17:39, Rob Herring <robh@kernel.org> a=20 ->> =E9crit : +>> +>> Le ven. 20 juil. 2018 à 17:39, Rob Herring <robh@kernel.org> a +>> écrit : >> > On Sat, Jul 14, 2018 at 03:50:08PM +0200, Paul Cercueil wrote: >> >> >> >> ->> >> Le sam. 14 juil. 2018 =E0 15:32, Alexandre Belloni ->> >> <alexandre.belloni@bootlin.com> a =E9crit : +>> >> Le sam. 14 juil. 2018 à 15:32, Alexandre Belloni +>> >> <alexandre.belloni@bootlin.com> a écrit : >> >> > On 14/07/2018 15:25:33+0200, Paul Cercueil wrote: >> >> > > Hi Alexandre, >> >> > > ->> >> > > Le sam. 14 juil. 2018 =E0 15:19, Alexandre Belloni ->> >> > > <alexandre.belloni@bootlin.com> a =E9crit : +>> >> > > Le sam. 14 juil. 2018 à 15:19, Alexandre Belloni +>> >> > > <alexandre.belloni@bootlin.com> a écrit : >> >> > > > Hello, >> >> > > > >> >> > > > On 13/07/2018 17:14:24+0200, Paul Cercueil wrote: >> >> > > > > The RTC in the JZ4725B works just like the one in the >> >> JZ4740. >> >> > > > > ->> >> > > > > The RTC in the JZ4760 and JZ4770 work just like the=20 +>> >> > > > > The RTC in the JZ4760 and JZ4770 work just like the >> one >> >> in the >> >> > > > > JZ4780. @@ -47,23 +47,23 @@ Le lun. 23 juil. 2018 =E0 16:18, Rob Herring <robh@kernel.org> a =E9crit : >> >> b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt >> >> > > > > index 41c7ae18fd7b..a9e821de84f2 100644 >> >> > > > > --- ->> >> > >=20 +>> >> > > >> a/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt >> >> > > > > +++ ->> >> > >=20 +>> >> > > >> b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt >> >> > > > > @@ -4,6 +4,9 @@ Required properties: >> >> > > > > >> >> > > > > - compatible: One of: ->> >> > > > > - "ingenic,jz4740-rtc" - for use with the JZ4740=20 +>> >> > > > > - "ingenic,jz4740-rtc" - for use with the JZ4740 >> SoC ->> >> > > > > + - "ingenic,jz4725b-rtc" - for use with the JZ4725B=20 +>> >> > > > > + - "ingenic,jz4725b-rtc" - for use with the JZ4725B >> SoC ->> >> > > > > + - "ingenic,jz4760-rtc" - for use with the JZ4760=20 +>> >> > > > > + - "ingenic,jz4760-rtc" - for use with the JZ4760 >> SoC ->> >> > > > > + - "ingenic,jz4770-rtc" - for use with the JZ4770=20 +>> >> > > > > + - "ingenic,jz4770-rtc" - for use with the JZ4770 >> SoC ->> >> > > > > - "ingenic,jz4780-rtc" - for use with the JZ4780=20 +>> >> > > > > - "ingenic,jz4780-rtc" - for use with the JZ4780 >> SoC >> >> > > > > - reg: Address range of rtc register set >> >> > > > > - interrupts: IRQ number for the alarm interrupt @@ -93,15 +93,15 @@ Le lun. 23 juil. 2018 =E0 16:18, Rob Herring <robh@kernel.org> a =E9crit : >> >> > > jz4740_rtc_reg_write(struct >> >> > > > > jz4740_rtc *rtc, size_t reg, >> >> > > > > { ->> >> > > > > int ret =3D 0; +>> >> > > > > int ret = 0; >> >> > > > > ->> >> > > > > - if (rtc->type >=3D ID_JZ4780) ->> >> > > > > + if (rtc->type >=3D ID_JZ4760) +>> >> > > > > - if (rtc->type >= ID_JZ4780) +>> >> > > > > + if (rtc->type >= ID_JZ4760) >> >> > > > ->> >> > > > This would avoid that change (and the test would=20 +>> >> > > > This would avoid that change (and the test would >> preferably >> >> be ->> >> > > > (rtc->type =3D=3D ID_JZ4780)) +>> >> > > > (rtc->type == ID_JZ4780)) >> >> > > >> >> > > That branch should be taken if the SoC is JZ4760, JZ4770 or >> >> JZ4780. @@ -110,53 +110,52 @@ Le lun. 23 juil. 2018 =E0 16:18, Rob Herring <robh@kernel.org> a =E9crit : >> >> > Sure but you can achieve that with only 2 ids... >> >> > >> >> > > ->> >> > > > > ret =3D jz4780_rtc_enable_write(rtc); ->> >> > > > > if (ret =3D=3D 0) ->> >> > > > > ret =3D jz4740_rtc_wait_write_ready(rtc); ->> >> > > > > @@ -300,6 +303,9 @@ static void=20 +>> >> > > > > 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=20 ->> jz4740_rtc_of_match[] =3D +>> >> > > > > static const struct of_device_id +>> jz4740_rtc_of_match[] = >> >> { ->> >> > > > > { .compatible =3D "ingenic,jz4740-rtc", .data =3D (void +>> >> > > > > { .compatible = "ingenic,jz4740-rtc", .data = (void >> >> > > *)ID_JZ4740 }, ->> >> > > > > + { .compatible =3D "ingenic,jz4725b-rtc", .data =3D (voi= -d +>> >> > > > > + { .compatible = "ingenic,jz4725b-rtc", .data = (void >> >> > > *)ID_JZ4725B >> >> > > > > }, ->> >> > > > > + { .compatible =3D "ingenic,jz4760-rtc", .data =3D (void +>> >> > > > > + { .compatible = "ingenic,jz4760-rtc", .data = (void >> >> > > *)ID_JZ4760 }, ->> >> > > > > + { .compatible =3D "ingenic,jz4770-rtc", .data =3D (void +>> >> > > > > + { .compatible = "ingenic,jz4770-rtc", .data = (void >> >> > > *)ID_JZ4770 }, >> >> > >> >> > By doing the correct mapping here e.g: >> >> > ->> >> > { .compatible =3D "ingenic,jz4725b-rtc", .data =3D (void=20 +>> >> > { .compatible = "ingenic,jz4725b-rtc", .data = (void >> *)ID_JZ4740 >> >> }, >> >> >> >> Not very pretty and future-proof if you ask me... >> > >> > Looks to me like this can be handled entirely in DT without driver ->> > changes like the other patches. Correct usage of compatible=20 +>> > changes like the other patches. Correct usage of compatible >> strings is ->> > what gives you future-proofing. And no driver change is better=20 +>> > what gives you future-proofing. And no driver change is better >> than >> > needless changing. ->>=20 ->> If I make e.g. the jz4760 and jz4770 use the jz4780 compatible=20 +>> +>> If I make e.g. the jz4760 and jz4770 use the jz4780 compatible >> string, >> but ->> then I want to implement a new feature that only exists on the=20 +>> then I want to implement a new feature that only exists on the >> jz4780, >> how >> can I do it without breaking everything? ->=20 +> > You specify both: ->=20 -> compatible =3D "ingenic,jz4760-rtc", "ingenic,jz4780-rtc"; ->=20 +> +> compatible = "ingenic,jz4760-rtc", "ingenic,jz4780-rtc"; +> > You match on the less specific compatible until you have features or > bugs to handle for the more specific compatible. In your case, you > would have to start matching on the 4760 and 4770 and not support the @@ -165,15 +164,14 @@ d Right, I didn't think about that. Thanks! ->> > It may look a bit wierd if 4780 is the fallback for 4770, but if=20 +>> > It may look a bit wierd if 4780 is the fallback for 4770, but if >> the >> > 4780 is older, then that actually makes sense. ->>=20 +>> >> The 4770 is older than the 4780. ->=20 +> > As I said, it's weird, but it will all still work. You could fix this > if you want. It would only matter if you had a old dtb with a new > kernel and care about that case working. ->=20 +> > Rob -= diff --git a/a/content_digest b/N1/content_digest index 5306d69..99d8c63 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -20,31 +20,31 @@ "b\0" "Hi,\n" "\n" - "Le lun. 23 juil. 2018 =E0 16:18, Rob Herring <robh@kernel.org> a =E9crit :\n" - "> On Fri, Jul 20, 2018 at 1:04 PM Paul Cercueil <paul@crapouillou.net>=20\n" + "Le lun. 23 juil. 2018 \303\240 16:18, Rob Herring <robh@kernel.org> a \303\251crit :\n" + "> On Fri, Jul 20, 2018 at 1:04 PM Paul Cercueil <paul@crapouillou.net> \n" "> wrote:\n" - ">>=20\n" + ">> \n" ">> Hi,\n" - ">>=20\n" - ">> Le ven. 20 juil. 2018 =E0 17:39, Rob Herring <robh@kernel.org> a=20\n" - ">> =E9crit :\n" + ">> \n" + ">> Le ven. 20 juil. 2018 \303\240 17:39, Rob Herring <robh@kernel.org> a \n" + ">> \303\251crit :\n" ">> > On Sat, Jul 14, 2018 at 03:50:08PM +0200, Paul Cercueil wrote:\n" ">> >>\n" ">> >>\n" - ">> >> Le sam. 14 juil. 2018 =E0 15:32, Alexandre Belloni\n" - ">> >> <alexandre.belloni@bootlin.com> a =E9crit :\n" + ">> >> Le sam. 14 juil. 2018 \303\240 15:32, Alexandre Belloni\n" + ">> >> <alexandre.belloni@bootlin.com> a \303\251crit :\n" ">> >> > On 14/07/2018 15:25:33+0200, Paul Cercueil wrote:\n" ">> >> > > Hi Alexandre,\n" ">> >> > >\n" - ">> >> > > Le sam. 14 juil. 2018 =E0 15:19, Alexandre Belloni\n" - ">> >> > > <alexandre.belloni@bootlin.com> a =E9crit :\n" + ">> >> > > Le sam. 14 juil. 2018 \303\240 15:19, Alexandre Belloni\n" + ">> >> > > <alexandre.belloni@bootlin.com> a \303\251crit :\n" ">> >> > > > Hello,\n" ">> >> > > >\n" ">> >> > > > On 13/07/2018 17:14:24+0200, Paul Cercueil wrote:\n" ">> >> > > > > The RTC in the JZ4725B works just like the one in the\n" ">> >> JZ4740.\n" ">> >> > > > >\n" - ">> >> > > > > The RTC in the JZ4760 and JZ4770 work just like the=20\n" + ">> >> > > > > The RTC in the JZ4760 and JZ4770 work just like the \n" ">> one\n" ">> >> in the\n" ">> >> > > > > JZ4780.\n" @@ -67,23 +67,23 @@ ">> >> b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt\n" ">> >> > > > > index 41c7ae18fd7b..a9e821de84f2 100644\n" ">> >> > > > > ---\n" - ">> >> > >=20\n" + ">> >> > > \n" ">> a/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt\n" ">> >> > > > > +++\n" - ">> >> > >=20\n" + ">> >> > > \n" ">> b/Documentation/devicetree/bindings/rtc/ingenic,jz4740-rtc.txt\n" ">> >> > > > > @@ -4,6 +4,9 @@ Required properties:\n" ">> >> > > > >\n" ">> >> > > > > - compatible: One of:\n" - ">> >> > > > > - \"ingenic,jz4740-rtc\" - for use with the JZ4740=20\n" + ">> >> > > > > - \"ingenic,jz4740-rtc\" - for use with the JZ4740 \n" ">> SoC\n" - ">> >> > > > > + - \"ingenic,jz4725b-rtc\" - for use with the JZ4725B=20\n" + ">> >> > > > > + - \"ingenic,jz4725b-rtc\" - for use with the JZ4725B \n" ">> SoC\n" - ">> >> > > > > + - \"ingenic,jz4760-rtc\" - for use with the JZ4760=20\n" + ">> >> > > > > + - \"ingenic,jz4760-rtc\" - for use with the JZ4760 \n" ">> SoC\n" - ">> >> > > > > + - \"ingenic,jz4770-rtc\" - for use with the JZ4770=20\n" + ">> >> > > > > + - \"ingenic,jz4770-rtc\" - for use with the JZ4770 \n" ">> SoC\n" - ">> >> > > > > - \"ingenic,jz4780-rtc\" - for use with the JZ4780=20\n" + ">> >> > > > > - \"ingenic,jz4780-rtc\" - for use with the JZ4780 \n" ">> SoC\n" ">> >> > > > > - reg: Address range of rtc register set\n" ">> >> > > > > - interrupts: IRQ number for the alarm interrupt\n" @@ -113,15 +113,15 @@ ">> >> > > jz4740_rtc_reg_write(struct\n" ">> >> > > > > jz4740_rtc *rtc, size_t reg,\n" ">> >> > > > > {\n" - ">> >> > > > > int ret =3D 0;\n" + ">> >> > > > > int ret = 0;\n" ">> >> > > > >\n" - ">> >> > > > > - if (rtc->type >=3D ID_JZ4780)\n" - ">> >> > > > > + if (rtc->type >=3D ID_JZ4760)\n" + ">> >> > > > > - if (rtc->type >= ID_JZ4780)\n" + ">> >> > > > > + if (rtc->type >= ID_JZ4760)\n" ">> >> > > >\n" - ">> >> > > > This would avoid that change (and the test would=20\n" + ">> >> > > > This would avoid that change (and the test would \n" ">> preferably\n" ">> >> be\n" - ">> >> > > > (rtc->type =3D=3D ID_JZ4780))\n" + ">> >> > > > (rtc->type == ID_JZ4780))\n" ">> >> > >\n" ">> >> > > That branch should be taken if the SoC is JZ4760, JZ4770 or\n" ">> >> JZ4780.\n" @@ -130,53 +130,52 @@ ">> >> > Sure but you can achieve that with only 2 ids...\n" ">> >> >\n" ">> >> > >\n" - ">> >> > > > > ret =3D jz4780_rtc_enable_write(rtc);\n" - ">> >> > > > > if (ret =3D=3D 0)\n" - ">> >> > > > > ret =3D jz4740_rtc_wait_write_ready(rtc);\n" - ">> >> > > > > @@ -300,6 +303,9 @@ static void=20\n" + ">> >> > > > > ret = jz4780_rtc_enable_write(rtc);\n" + ">> >> > > > > if (ret == 0)\n" + ">> >> > > > > ret = jz4740_rtc_wait_write_ready(rtc);\n" + ">> >> > > > > @@ -300,6 +303,9 @@ static void \n" ">> jz4740_rtc_power_off(void)\n" ">> >> > > > >\n" - ">> >> > > > > static const struct of_device_id=20\n" - ">> jz4740_rtc_of_match[] =3D\n" + ">> >> > > > > static const struct of_device_id \n" + ">> jz4740_rtc_of_match[] =\n" ">> >> {\n" - ">> >> > > > > { .compatible =3D \"ingenic,jz4740-rtc\", .data =3D (void\n" + ">> >> > > > > { .compatible = \"ingenic,jz4740-rtc\", .data = (void\n" ">> >> > > *)ID_JZ4740 },\n" - ">> >> > > > > + { .compatible =3D \"ingenic,jz4725b-rtc\", .data =3D (voi=\n" - "d\n" + ">> >> > > > > + { .compatible = \"ingenic,jz4725b-rtc\", .data = (void\n" ">> >> > > *)ID_JZ4725B\n" ">> >> > > > > },\n" - ">> >> > > > > + { .compatible =3D \"ingenic,jz4760-rtc\", .data =3D (void\n" + ">> >> > > > > + { .compatible = \"ingenic,jz4760-rtc\", .data = (void\n" ">> >> > > *)ID_JZ4760 },\n" - ">> >> > > > > + { .compatible =3D \"ingenic,jz4770-rtc\", .data =3D (void\n" + ">> >> > > > > + { .compatible = \"ingenic,jz4770-rtc\", .data = (void\n" ">> >> > > *)ID_JZ4770 },\n" ">> >> >\n" ">> >> > By doing the correct mapping here e.g:\n" ">> >> >\n" - ">> >> > { .compatible =3D \"ingenic,jz4725b-rtc\", .data =3D (void=20\n" + ">> >> > { .compatible = \"ingenic,jz4725b-rtc\", .data = (void \n" ">> *)ID_JZ4740\n" ">> >> },\n" ">> >>\n" ">> >> Not very pretty and future-proof if you ask me...\n" ">> >\n" ">> > Looks to me like this can be handled entirely in DT without driver\n" - ">> > changes like the other patches. Correct usage of compatible=20\n" + ">> > changes like the other patches. Correct usage of compatible \n" ">> strings is\n" - ">> > what gives you future-proofing. And no driver change is better=20\n" + ">> > what gives you future-proofing. And no driver change is better \n" ">> than\n" ">> > needless changing.\n" - ">>=20\n" - ">> If I make e.g. the jz4760 and jz4770 use the jz4780 compatible=20\n" + ">> \n" + ">> If I make e.g. the jz4760 and jz4770 use the jz4780 compatible \n" ">> string,\n" ">> but\n" - ">> then I want to implement a new feature that only exists on the=20\n" + ">> then I want to implement a new feature that only exists on the \n" ">> jz4780,\n" ">> how\n" ">> can I do it without breaking everything?\n" - ">=20\n" + "> \n" "> You specify both:\n" - ">=20\n" - "> compatible =3D \"ingenic,jz4760-rtc\", \"ingenic,jz4780-rtc\";\n" - ">=20\n" + "> \n" + "> compatible = \"ingenic,jz4760-rtc\", \"ingenic,jz4780-rtc\";\n" + "> \n" "> You match on the less specific compatible until you have features or\n" "> bugs to handle for the more specific compatible. In your case, you\n" "> would have to start matching on the 4760 and 4770 and not support the\n" @@ -185,17 +184,16 @@ "Right, I didn't think about that.\n" "Thanks!\n" "\n" - ">> > It may look a bit wierd if 4780 is the fallback for 4770, but if=20\n" + ">> > It may look a bit wierd if 4780 is the fallback for 4770, but if \n" ">> the\n" ">> > 4780 is older, then that actually makes sense.\n" - ">>=20\n" + ">> \n" ">> The 4770 is older than the 4780.\n" - ">=20\n" + "> \n" "> As I said, it's weird, but it will all still work. You could fix this\n" "> if you want. It would only matter if you had a old dtb with a new\n" "> kernel and care about that case working.\n" - ">=20\n" - "> Rob\n" - = + "> \n" + > Rob -cf6b046a1e82e4e35c1745d3f6c2aae6a612e860adf3cdb717e76fc7a3b18bab +6c44939cbd3c8c1a784dca894e42fdf2d00a485a681c907f4cb8a9f6f89daf52
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.