| Message ID | 20231006214433.339688-1-robh@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | ata: xgene: Use of_device_get_match_data() | expand |
On Fri, Oct 06, 2023 at 04:44:33PM -0500, Rob Herring wrote: > Use preferred of_device_get_match_data() instead of of_match_device() to > get the driver match data. With this, adjust the includes to explicitly > include the correct headers. > > Signed-off-by: Rob Herring <robh@kernel.org> > --- > Perhaps device_get_match_data() could be used here and all the ACPI > specific code dropped, but not sure if all the ACPI code is really > necessary. > --- > drivers/ata/ahci_xgene.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c > index ccef5e63bdf9..81a1d838c0fc 100644 > --- a/drivers/ata/ahci_xgene.c > +++ b/drivers/ata/ahci_xgene.c > @@ -13,9 +13,7 @@ > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/ahci_platform.h> > -#include <linux/of_address.h> > -#include <linux/of_device.h> > -#include <linux/of_irq.h> > +#include <linux/of.h> > #include <linux/phy/phy.h> > #include "ahci.h" > > @@ -735,7 +733,6 @@ static int xgene_ahci_probe(struct platform_device *pdev) > struct ahci_host_priv *hpriv; > struct xgene_ahci_context *ctx; > struct resource *res; > - const struct of_device_id *of_devid; > enum xgene_ahci_version version = XGENE_AHCI_V1; > const struct ata_port_info *ppi[] = { &xgene_ahci_v1_port_info, > &xgene_ahci_v2_port_info }; > @@ -778,10 +775,8 @@ static int xgene_ahci_probe(struct platform_device *pdev) > ctx->csr_mux = csr; > } > > - of_devid = of_match_device(xgene_ahci_of_match, dev); > - if (of_devid) { > - if (of_devid->data) > - version = (unsigned long) of_devid->data; > + if (dev->of_node) { > + version = (enum xgene_ahci_version)of_device_get_match_data(dev); > } Nit: braces can be removed. > #ifdef CONFIG_ACPI > else { > -- > 2.40.1 > With or without nit fixed: Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
On Mon, Oct 9, 2023 at 9:41 AM Niklas Cassel <Niklas.Cassel@wdc.com> wrote: > > On Fri, Oct 06, 2023 at 04:44:33PM -0500, Rob Herring wrote: > > Use preferred of_device_get_match_data() instead of of_match_device() to > > get the driver match data. With this, adjust the includes to explicitly > > include the correct headers. > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > Perhaps device_get_match_data() could be used here and all the ACPI > > specific code dropped, but not sure if all the ACPI code is really > > necessary. > > --- > > drivers/ata/ahci_xgene.c | 11 +++-------- > > 1 file changed, 3 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c > > index ccef5e63bdf9..81a1d838c0fc 100644 > > --- a/drivers/ata/ahci_xgene.c > > +++ b/drivers/ata/ahci_xgene.c > > @@ -13,9 +13,7 @@ > > #include <linux/module.h> > > #include <linux/platform_device.h> > > #include <linux/ahci_platform.h> > > -#include <linux/of_address.h> > > -#include <linux/of_device.h> > > -#include <linux/of_irq.h> > > +#include <linux/of.h> > > #include <linux/phy/phy.h> > > #include "ahci.h" > > > > @@ -735,7 +733,6 @@ static int xgene_ahci_probe(struct platform_device *pdev) > > struct ahci_host_priv *hpriv; > > struct xgene_ahci_context *ctx; > > struct resource *res; > > - const struct of_device_id *of_devid; > > enum xgene_ahci_version version = XGENE_AHCI_V1; > > const struct ata_port_info *ppi[] = { &xgene_ahci_v1_port_info, > > &xgene_ahci_v2_port_info }; > > @@ -778,10 +775,8 @@ static int xgene_ahci_probe(struct platform_device *pdev) > > ctx->csr_mux = csr; > > } > > > > - of_devid = of_match_device(xgene_ahci_of_match, dev); > > - if (of_devid) { > > - if (of_devid->data) > > - version = (unsigned long) of_devid->data; > > + if (dev->of_node) { > > + version = (enum xgene_ahci_version)of_device_get_match_data(dev); > > } > > Nit: braces can be removed. > > > #ifdef CONFIG_ACPI > > else { Coding style says otherwise when the 'else' has braces. Rob
On Mon, Oct 09, 2023 at 10:19:54AM -0500, Rob Herring wrote: > On Mon, Oct 9, 2023 at 9:41 AM Niklas Cassel <Niklas.Cassel@wdc.com> wrote: > > > > On Fri, Oct 06, 2023 at 04:44:33PM -0500, Rob Herring wrote: > > > Use preferred of_device_get_match_data() instead of of_match_device() to > > > get the driver match data. With this, adjust the includes to explicitly > > > include the correct headers. > > > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > > --- > > > Perhaps device_get_match_data() could be used here and all the ACPI > > > specific code dropped, but not sure if all the ACPI code is really > > > necessary. > > > --- > > > drivers/ata/ahci_xgene.c | 11 +++-------- > > > 1 file changed, 3 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c > > > index ccef5e63bdf9..81a1d838c0fc 100644 > > > --- a/drivers/ata/ahci_xgene.c > > > +++ b/drivers/ata/ahci_xgene.c > > > @@ -13,9 +13,7 @@ > > > #include <linux/module.h> > > > #include <linux/platform_device.h> > > > #include <linux/ahci_platform.h> > > > -#include <linux/of_address.h> > > > -#include <linux/of_device.h> > > > -#include <linux/of_irq.h> > > > +#include <linux/of.h> > > > #include <linux/phy/phy.h> > > > #include "ahci.h" > > > > > > @@ -735,7 +733,6 @@ static int xgene_ahci_probe(struct platform_device *pdev) > > > struct ahci_host_priv *hpriv; > > > struct xgene_ahci_context *ctx; > > > struct resource *res; > > > - const struct of_device_id *of_devid; > > > enum xgene_ahci_version version = XGENE_AHCI_V1; > > > const struct ata_port_info *ppi[] = { &xgene_ahci_v1_port_info, > > > &xgene_ahci_v2_port_info }; > > > @@ -778,10 +775,8 @@ static int xgene_ahci_probe(struct platform_device *pdev) > > > ctx->csr_mux = csr; > > > } > > > > > > - of_devid = of_match_device(xgene_ahci_of_match, dev); > > > - if (of_devid) { > > > - if (of_devid->data) > > > - version = (unsigned long) of_devid->data; > > > + if (dev->of_node) { > > > + version = (enum xgene_ahci_version)of_device_get_match_data(dev); > > > } > > > > Nit: braces can be removed. > > > > > #ifdef CONFIG_ACPI > > > else { > > Coding style says otherwise when the 'else' has braces. Hello Rob, I know what the coding style says when the 'else' has braces. I simply missed that there was an else with braces, since there was a closing brace on a line by itself. The coding style says that the closing brace should be on the same line as the 'else': https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces But I guess that isn't followed because of the #ifdef CONFIG_ACPI. I guess a better option would be to use if (IS_ENABLED(CONFIG_ACPI)) instead of #ifdef CONFIG_ACPI, but that is orthogonal to your change, so let's ignore my nit. I've given my R-b tag on the patch already. Kind regards, Niklas
On 10/7/23 06:44, Rob Herring wrote: > Use preferred of_device_get_match_data() instead of of_match_device() to > get the driver match data. With this, adjust the includes to explicitly > include the correct headers. > > Signed-off-by: Rob Herring <robh@kernel.org> Applied to for-6.7. Thanks ! > --- > Perhaps device_get_match_data() could be used here and all the ACPI > specific code dropped, but not sure if all the ACPI code is really > necessary. Will have a look at that.
diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c index ccef5e63bdf9..81a1d838c0fc 100644 --- a/drivers/ata/ahci_xgene.c +++ b/drivers/ata/ahci_xgene.c @@ -13,9 +13,7 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <linux/ahci_platform.h> -#include <linux/of_address.h> -#include <linux/of_device.h> -#include <linux/of_irq.h> +#include <linux/of.h> #include <linux/phy/phy.h> #include "ahci.h" @@ -735,7 +733,6 @@ static int xgene_ahci_probe(struct platform_device *pdev) struct ahci_host_priv *hpriv; struct xgene_ahci_context *ctx; struct resource *res; - const struct of_device_id *of_devid; enum xgene_ahci_version version = XGENE_AHCI_V1; const struct ata_port_info *ppi[] = { &xgene_ahci_v1_port_info, &xgene_ahci_v2_port_info }; @@ -778,10 +775,8 @@ static int xgene_ahci_probe(struct platform_device *pdev) ctx->csr_mux = csr; } - of_devid = of_match_device(xgene_ahci_of_match, dev); - if (of_devid) { - if (of_devid->data) - version = (unsigned long) of_devid->data; + if (dev->of_node) { + version = (enum xgene_ahci_version)of_device_get_match_data(dev); } #ifdef CONFIG_ACPI else {
Use preferred of_device_get_match_data() instead of of_match_device() to get the driver match data. With this, adjust the includes to explicitly include the correct headers. Signed-off-by: Rob Herring <robh@kernel.org> --- Perhaps device_get_match_data() could be used here and all the ACPI specific code dropped, but not sure if all the ACPI code is really necessary. --- drivers/ata/ahci_xgene.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)