Thread (8 messages) flat view 8 messages, 2 authors, 2015-05-04
DORMANTno replies

[PATCH 1/2] stmac: add dwmac glue for NXP 18xx/43xx family

From: Joachim Eastwood <hidden>
Date: 2015-05-04 19:27:53
Also in: netdev

On 4 May 2015 at 20:46, Arnd Bergmann [off-list ref] wrote:
On Monday 04 May 2015 00:14:11 Joachim Eastwood wrote:
=> >It should be fairly straightforward to split the probe function
quoted
quoted
into two and export a function that takes a device and a stmmac_of_data
pointer as arguments, and declare a module_platform_driver in your
code.
How about something like the patch below. All it does is to play a
little trick with the of_match_device in the dt config function and
export the probe/remove/pm stuff for the driver.

The dwmac-lpc18xx would then become something like this:

static const struct stmmac_of_data lpc18xx_dwmac_data = {
      .has_gmac = 1,
      .setup = lpc18xx_dwmac_setup,
      .init = lpc18xx_dwmac_init,
};

static const struct of_device_id lpc18xx_dwmac_match[] = {
      { .compatible = "nxp,lpc1850-dwmac", .data = &lpc18xx_dwmac_data },
      { }
};
MODULE_DEVICE_TABLE(of, lpc18xx_dwmac_match);

static struct platform_driver lpc18xx_dwmac_driver = {
      .probe  = stmmac_pltfr_probe,
      .remove = stmmac_pltfr_remove,
      .driver = {
              .name           = "lpc18xx-dwmac",
              .pm             = &stmmac_pltfr_pm_ops,
              .of_match_table = lpc18xx_dwmac_match,
      },
};
module_platform_driver(lpc18xx_dwmac_driver);

All though this seem to work, stmmac_platform.c could benefit from
some refactoring. But this patch and then fixing the other DT users
could be a first step.
Sounds good, yes.
quoted
-     device = of_match_device(stmmac_dt_ids, &pdev->dev);
+     device = of_match_device(dev->driver->of_match_table, dev);
      if (!device)
              return -ENODEV;
Ah, that is a nice trick to avoid passing the various match tables
from a lot of duplicated probe functions.

I wonder if we could generalize that for use by any driver and
introduce a common helper
I realised that and posted kinda of a RFC to the devicetree list yesterday.
http://marc.info/?l=devicetree&m=143068795621692&w=2
void *of_platform_match_data(struct device *dev)
{
        struct of_device_id *id;

        if (!dev || !dev->of_node)
                return NULL;

        id = of_match_device(dev->driver->of_match_table, dev);
        if (!id)
                return NULL;

        return id->data;
}
EXPORT_SYMBOL_GPL(of_platform_match_data);
Almost the same as your proposal except for the dev pointer checking
and the name.
I think that could save a few lines from many drivers, and it had not
occurred to me that we can take this shortcut.

The rest of your patch also looks great to me.
Great, I'll put together a new series of patches.

regards,
Joachim
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help