| Message ID | 1483083470-15779-1-git-send-email-green.hu@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 12/29/2016 11:37 PM, Greentime Hu wrote:
> Signed-off-by: Greentime Hu <green.hu@gmail.com>
This is not enough, you need to add a Device Tree binding document under
Documentation/devicetree/bindings/net/ which documents this compatible
string, as well as additional properties that may be required to
describe this hardware block.
On Saturday, December 31, 2016 10:48:39 AM CET Florian Fainelli wrote: > > On 12/29/2016 11:37 PM, Greentime Hu wrote: > > Signed-off-by: Greentime Hu <green.hu@gmail.com> > > This is not enough, you need to add a Device Tree binding document under > Documentation/devicetree/bindings/net/ which documents this compatible > string, as well as additional properties that may be required to > describe this hardware block. We already have Documentation/devicetree/bindings/net/moxa,moxart-mac.txt for the same hardware (though used by a different driver). I'd suggest renaming that one to a more generic file name and adding the new compatible string there. Aside from that, every patch should also have a changelog comment. Arnd
On Tuesday, January 3, 2017 2:05:47 PM CET Greentime Hu wrote: > I am not sure if atmac and moxa-art are exactly hardware compatible though > they are based on faraday ftmac. > It may be better if we use 2 different device tree binding documents to > describe for these 2 different drivers to use. They are probably slightly different, but close enough to have the same binding document, as there is no technical reason to have two separate drivers for them. The binding should be about the hardware type, not the way that Linux currently implements the drivers. Arnd
On Wednesday, January 4, 2017 9:49:51 AM CET Greentime Hu wrote: > On Tue, Jan 3, 2017 at 9:24 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > > On Tuesday, January 3, 2017 2:05:47 PM CET Greentime Hu wrote: > > > I am not sure if atmac and moxa-art are exactly hardware compatible > > though > > > they are based on faraday ftmac. > > > It may be better if we use 2 different device tree binding documents to > > > describe for these 2 different drivers to use. > > > > They are probably slightly different, but close enough to have the same > > binding document, as there is no technical reason to have two separate > > drivers for them. The binding should be about the hardware type, not the > > way that Linux currently implements the drivers. > > > > Arnd > > > > > OK. > > How about this? > > rename > Documentation/devicetree/bindings/net/moxa,moxart-mac.txt > to > Documentation/devicetree/bindings/net/faraday,ftmac.txt > > and the content to > Faraday Ethernet Controller Sounds good. Note that you can use 'git patch -M' to produce this as a renaming patch. > > Required properties: > > - compatible : Must be "moxa,moxart-mac" or "andestech,atmac" or > "faraday,ftmac" I'd write this as compatible: Must contain "faraday,ftmac", as well as one of the SoC specific identifiers: "andestec,atmac" "moxa,moxart-mac" This makes it easier to extend, plus it makes the generic string mandatory. Arnd
diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c index dce5f7b..5d70ee9 100644 --- a/drivers/net/ethernet/faraday/ftmac100.c +++ b/drivers/net/ethernet/faraday/ftmac100.c @@ -1172,11 +1172,17 @@ static int __exit ftmac100_remove(struct platform_device *pdev) return 0; } +static const struct of_device_id ftmac100_of_ids[] = { + { .compatible = "andestech,atmac100" }, + { } +}; + static struct platform_driver ftmac100_driver = { .probe = ftmac100_probe, .remove = __exit_p(ftmac100_remove), .driver = { .name = DRV_NAME, + .of_match_table = ftmac100_of_ids }, }; @@ -1200,3 +1206,4 @@ static void __exit ftmac100_exit(void) MODULE_AUTHOR("Po-Yu Chuang <ratbert@faraday-tech.com>"); MODULE_DESCRIPTION("FTMAC100 driver"); MODULE_LICENSE("GPL"); +MODULE_DEVICE_TABLE(of, ftmac100_of_ids);
Signed-off-by: Greentime Hu <green.hu@gmail.com> --- Changes in v3: - Nothing changed in this patch but I have committed andestech to vendor-prefixes.txt. drivers/net/ethernet/faraday/ftmac100.c | 7 +++++++ 1 file changed, 7 insertions(+)