| Message ID | 20200904131152.17390-1-paul@crapouillou.net |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [1/3] dt-bindings: i2c: ingenic: Add compatible string for the JZ4770 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success |
On Fri, Sep 04, 2020 at 03:11:52PM +0200, Paul Cercueil wrote: > CONFIG_OF is selected by CONFIG_MACH_INGENIC, therefore we don't need to > handle the case where Device Tree is not supported. What about COMPILE_TEST? If not supported, why not? > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > drivers/i2c/busses/i2c-jz4780.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c > index ed2ec86f6f1a..cb4a25ebb890 100644 > --- a/drivers/i2c/busses/i2c-jz4780.c > +++ b/drivers/i2c/busses/i2c-jz4780.c > @@ -857,7 +857,7 @@ static struct platform_driver jz4780_i2c_driver = { > .remove = jz4780_i2c_remove, > .driver = { > .name = "jz4780-i2c", > - .of_match_table = of_match_ptr(jz4780_i2c_of_matches), > + .of_match_table = jz4780_i2c_of_matches, > }, > }; > > -- > 2.28.0 >
On Fri, 04 Sep 2020 15:11:50 +0200, Paul Cercueil wrote: > The I2C controller in the JZ4770 SoC seems to work the exact same as in > the JZ4780 SoC. > > We could use "ingenic,jz4780-i2c" as a fallback string in the Device > Tree, but that would be awkward, since the JZ4780 is newer. Instead, > add a "ingenic,jz4770-i2c" string and use it as fallback for the > "ingenic,jz4780-i2c" string. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > .../devicetree/bindings/i2c/ingenic,i2c.yaml | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > Reviewed-by: Rob Herring <robh@kernel.org>
Hi Rob, Le lun. 14 sept. 2020 à 16:12, Rob Herring <robh@kernel.org> a écrit : > On Fri, Sep 04, 2020 at 03:11:52PM +0200, Paul Cercueil wrote: >> CONFIG_OF is selected by CONFIG_MACH_INGENIC, therefore we don't >> need to >> handle the case where Device Tree is not supported. > > What about COMPILE_TEST? If not supported, why not? What about it? It will still compile fine with COMPILE_TEST. -Paul >> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> --- >> drivers/i2c/busses/i2c-jz4780.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/i2c/busses/i2c-jz4780.c >> b/drivers/i2c/busses/i2c-jz4780.c >> index ed2ec86f6f1a..cb4a25ebb890 100644 >> --- a/drivers/i2c/busses/i2c-jz4780.c >> +++ b/drivers/i2c/busses/i2c-jz4780.c >> @@ -857,7 +857,7 @@ static struct platform_driver jz4780_i2c_driver >> = { >> .remove = jz4780_i2c_remove, >> .driver = { >> .name = "jz4780-i2c", >> - .of_match_table = of_match_ptr(jz4780_i2c_of_matches), >> + .of_match_table = jz4780_i2c_of_matches, >> }, >> }; >> >> -- >> 2.28.0 >>
On Tue, Sep 15, 2020 at 4:07 AM Paul Cercueil <paul@crapouillou.net> wrote: > > Hi Rob, > > Le lun. 14 sept. 2020 à 16:12, Rob Herring <robh@kernel.org> a écrit : > > On Fri, Sep 04, 2020 at 03:11:52PM +0200, Paul Cercueil wrote: > >> CONFIG_OF is selected by CONFIG_MACH_INGENIC, therefore we don't > >> need to > >> handle the case where Device Tree is not supported. > > > > What about COMPILE_TEST? If not supported, why not? > > What about it? It will still compile fine with COMPILE_TEST. CONFIG_OF could be disabled in that case, so the above reasoning doesn't hold. Rob
Le mar. 15 sept. 2020 à 10:03, Rob Herring <robh@kernel.org> a écrit : > On Tue, Sep 15, 2020 at 4:07 AM Paul Cercueil <paul@crapouillou.net> > wrote: >> >> Hi Rob, >> >> Le lun. 14 sept. 2020 à 16:12, Rob Herring <robh@kernel.org> a >> écrit : >> > On Fri, Sep 04, 2020 at 03:11:52PM +0200, Paul Cercueil wrote: >> >> CONFIG_OF is selected by CONFIG_MACH_INGENIC, therefore we don't >> >> need to >> >> handle the case where Device Tree is not supported. >> > >> > What about COMPILE_TEST? If not supported, why not? >> >> What about it? It will still compile fine with COMPILE_TEST. > > CONFIG_OF could be disabled in that case, so the above reasoning > doesn't hold. > CONFIG_OF can be disabled in that case, correct, but why should we care? The driver will still compile fine. -Paul
On Tue, Sep 15, 2020 at 10:07 AM Paul Cercueil <paul@crapouillou.net> wrote: > > > > Le mar. 15 sept. 2020 à 10:03, Rob Herring <robh@kernel.org> a écrit : > > On Tue, Sep 15, 2020 at 4:07 AM Paul Cercueil <paul@crapouillou.net> > > wrote: > >> > >> Hi Rob, > >> > >> Le lun. 14 sept. 2020 à 16:12, Rob Herring <robh@kernel.org> a > >> écrit : > >> > On Fri, Sep 04, 2020 at 03:11:52PM +0200, Paul Cercueil wrote: > >> >> CONFIG_OF is selected by CONFIG_MACH_INGENIC, therefore we don't > >> >> need to > >> >> handle the case where Device Tree is not supported. > >> > > >> > What about COMPILE_TEST? If not supported, why not? > >> > >> What about it? It will still compile fine with COMPILE_TEST. > > > > CONFIG_OF could be disabled in that case, so the above reasoning > > doesn't hold. > > > > CONFIG_OF can be disabled in that case, correct, but why should we > care? The driver will still compile fine. Indeed, because jz4780_i2c_of_matches isn't within a CONFIG_OF ifdef as is sometimes done and is when you need of_match_ptr(). IMO, the commit msg should have something like "The driver is only used with CONFIG_OF enabled, so of_match_ptr() is not necessary. jz4780_i2c_of_matches is always defined." Rob
> Indeed, because jz4780_i2c_of_matches isn't within a CONFIG_OF ifdef > as is sometimes done and is when you need of_match_ptr(). IMO, the > commit msg should have something like "The driver is only used with > CONFIG_OF enabled, so of_match_ptr() is not necessary. > jz4780_i2c_of_matches is always defined." I think the commit message says that good enough.
On Fri, Sep 04, 2020 at 03:11:50PM +0200, Paul Cercueil wrote: > The I2C controller in the JZ4770 SoC seems to work the exact same as in > the JZ4780 SoC. > > We could use "ingenic,jz4780-i2c" as a fallback string in the Device > Tree, but that would be awkward, since the JZ4780 is newer. Instead, > add a "ingenic,jz4770-i2c" string and use it as fallback for the > "ingenic,jz4780-i2c" string. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> Applied to for-next, thanks!
On Fri, Sep 04, 2020 at 03:11:51PM +0200, Paul Cercueil wrote: > The I2C controller in the JZ4770 SoC seems to work the exact same as in > the JZ4780 SoC. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> Applied to for-next, thanks!
On Fri, Sep 04, 2020 at 03:11:52PM +0200, Paul Cercueil wrote: > CONFIG_OF is selected by CONFIG_MACH_INGENIC, therefore we don't need to > handle the case where Device Tree is not supported. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> Applied to for-next, thanks!
diff --git a/Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml b/Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml index 682ed1bbf5c6..0e7b4b8a7e48 100644 --- a/Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml +++ b/Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml @@ -17,9 +17,13 @@ properties: pattern: "^i2c@[0-9a-f]+$" compatible: - enum: - - ingenic,jz4780-i2c - - ingenic,x1000-i2c + oneOf: + - enum: + - ingenic,jz4770-i2c + - ingenic,x1000-i2c + - items: + - const: ingenic,jz4780-i2c + - const: ingenic,jz4770-i2c reg: maxItems: 1 @@ -60,7 +64,7 @@ examples: #include <dt-bindings/dma/jz4780-dma.h> #include <dt-bindings/interrupt-controller/irq.h> i2c@10054000 { - compatible = "ingenic,jz4780-i2c"; + compatible = "ingenic,jz4780-i2c", "ingenic,jz4770-i2c"; #address-cells = <1>; #size-cells = <0>; reg = <0x10054000 0x1000>;
The I2C controller in the JZ4770 SoC seems to work the exact same as in the JZ4780 SoC. We could use "ingenic,jz4780-i2c" as a fallback string in the Device Tree, but that would be awkward, since the JZ4780 is newer. Instead, add a "ingenic,jz4770-i2c" string and use it as fallback for the "ingenic,jz4780-i2c" string. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- .../devicetree/bindings/i2c/ingenic,i2c.yaml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)