New function, 'of_dev_get_platdata()'
- provides unified handling of getting device platform data
- supports DT and non-DT(legacy) cases
- removes duplicated code from each driver
- keeps driver specific code simple in each driver
Issues
------
On loading a driver, the driver tries to get the platform data.
* Check conditions prior to parsing the DT
You can see various/not general way of checking the platform data.
case 1) driver checks whether the platform data is null or not.
foo_platform_data *pdata = dev_get_platdata(dev);
if (!pdata) {
pdata = foo_parse_dt();
if (IS_ERR(pdata))
return PTR_ERR(pdata);
}
case 2) driver checks whether 'of_node' exists or not.
if (dev.of_node) {
pdata = foo_parse_dt();
if (IS_ERR(pdata))
return PTR_ERR(pdata);
}
case 3) check both
if (pdata) {
copy pdata into driver private data
} else if (dev.of_node) {
pdata = foo_parse_dt();
}
Check conditions are very depend on the driver, but it can be unified.
of_dev_get_platdata() provides unified handling. So the driver can reduce
if-condition statements.
* Memory allocation in each parser
In most cases, driver allocates the platform data inside its DT parser.
static struct foo_platform_data *foo_parse_dt()
{
allocates memory for generating platform data.
parsing DT properties and copy them into the platform data.
}
of_dev_get_platdata() allocates the platform data internally.
And it calls back to the driver specific parser function. It removes
memory allocation code in each driver.
* Two types of parser definition
Many drivers implement DT parser in two cases, '#ifdef CONFIG_OF' and
'#else'.
#ifdef CONFIG_OF
static struct foo_platform_data *foo_parse_dt()
{
(snip)
}
#else
static struct foo_platform_data *foo_parse_dt()
{
return NULL;
}
#endif
of_dev_get_platdata() supports both cases. It removes few lines of code
in each driver.
What of_dev_get_platdata() does
-------------------------------
if CONFIG_OF is not defined, return legacy code, 'dev_get_platdata()'.
if platform data exists, just return it.
if platform data is null, check the 'of node'.
if of_node is null, then return NULL.
Otherwise, allocates memory for platform data.
Call back to the driver(caller) with allocated platform data and
private data if needed.
Then, driver will parse the DT internally and handle errors.
Examples
--------
Following patches are examples.
drivers/input/touchscreen/mms114.c
drivers/mfd/tps65910.c
drivers/usb/musb/ux500.c
Driver list
-----------
of_dev_get_platdata() can be applied into files below. You may find more
if you're interested in this. :)
drivers/dma/ste_dma40.c
drivers/gpio/gpio-rcar.c
drivers/gpu/drm/exynos/exynos_dp_core.c
drivers/gpu/drm/exynos/exynos_hdmi.c
drivers/hwmon/ntc_thermistor.c
drivers/i2c/busses/i2c-s3c2410.c
drivers/iio/frequency/adf4350.c
drivers/input/keyboard/matrix_keypad.c
drivers/input/keyboard/samsung-keypad.c
drivers/input/misc/drv260x.c
drivers/input/misc/regulator-haptic.c
drivers/input/misc/rotary_encoder.c
drivers/input/touchscreen/atmel_mxt_ts.c
drivers/input/touchscreen/auo-pixcir-ts.c
drivers/input/touchscreen/bu21013_ts.c
drivers/input/touchscreen/mms114.c
drivers/input/touchscreen/pixcir_i2c_ts.c
drivers/input/touchscreen/zforce_ts.c
drivers/leds/leds-lp5521.c
drivers/leds/leds-lp5523.c
drivers/leds/leds-lp5562.c
drivers/leds/leds-lp55xx-common.c
drivers/leds/leds-lp8501.c
drivers/leds/leds-mc13783.c
drivers/leds/leds-pca963x.c
drivers/leds/leds-tca6507.c
drivers/mfd/max8997.c
drivers/mfd/max8998.c
drivers/mfd/sec-core.c
drivers/mfd/tps6586x.c
drivers/mfd/tps65910.c
drivers/misc/lis3lv02d/lis3lv02d.c
drivers/mmc/host/davinci_mmc.c
drivers/mmc/host/dw_mmc.c
drivers/mmc/host/sdhci-s3c.c
drivers/mtd/devices/spear_smi.c
drivers/mtd/nand/fsmc_nand.c
drivers/mtd/nand/lpc32xx_mlc.c
drivers/mtd/nand/lpc32xx_slc.c
drivers/mtd/nand/sh_flctl.c
drivers/net/can/sja1000/sja1000_platform.c
drivers/net/ethernet/davicom/dm9000.c
drivers/net/ethernet/marvell/mv643xx_eth.c
drivers/net/ethernet/renesas/sh_eth.c
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
drivers/power/bq24735-charger.c
drivers/power/gpio-charger.c
drivers/power/sbs-battery.c
drivers/power/tps65090-charger.c
drivers/power/twl4030_charger.c
drivers/pwm/pwm-lp3943.c
drivers/pwm/pwm-samsung.c
drivers/spi/spi-sh-msiof.c
drivers/spi/spi/spi-s3c64xx.c
drivers/thermal/db8500_thermal.c
drivers/usb/misc/usb3503.c
drivers/usb/musb/omap2430.c
drivers/usb/musb/ux500.c
drivers/usb/renesas_usbhs/common.c
drivers/video/fbdev/simplefb.c
drivers/video/backlight/lp855x_bl.c
drivers/video/backlight/pwm_bl.c
drivers/video/backlight/sky81452-backlight.c
drivers/video/backlight/tps65217_bl.c
This is the RFC, so I would like to get some feedback prior to patching all
drivers. Any comments are welcome.
Cc: Dmitry Torokhov <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Samuel Ortiz <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: [email protected]
Cc: [email protected]
Milo Kim (4):
of: introduce of_dev_get_platdata()
input: touchscree: mms114: use of_dev_get_platdata()
mfd: tps65910: use of_dev_get_platdata()
usb: musb: use of_dev_get_platdata()
drivers/input/touchscreen/mms114.c | 34 ++++++++------------------
drivers/mfd/tps65910.c | 49 +++++++++++++-------------------------
drivers/of/device.c | 46 +++++++++++++++++++++++++++++++++++
drivers/usb/musb/ux500.c | 40 +++++++++++++------------------
include/linux/of_device.h | 12 ++++++++++
5 files changed, 100 insertions(+), 81 deletions(-)
--
1.9.1
of_dev_get_platdata()
- provides unified handling of getting device platform data
- supports DT and non-DT(legacy) cases
if CONFIG_OF is not defined, then returns legacy function,
'dev_get_platdata()'.
- removes duplicated code from each driver
- keeps driver specific code simple in each driver
Parser function - of_parse_dt_fn()
Caller(aka driver) gets allocated platform data and optional private
data. Then, it will parse the DT and copy properties into allocated
platform data and use the private data if needed.
Cc: Dmitry Torokhov <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Samuel Ortiz <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Milo Kim <[email protected]>
---
drivers/of/device.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of_device.h | 12 ++++++++++++
2 files changed, 58 insertions(+)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 8b91ea2..5793ba0 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -286,3 +286,49 @@ int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env)
return 0;
}
+
+/**
+ * of_dev_get_platdata - get the platform data.
+ * @dev: device to get the platform data.
+ * @size: size of the platform data.
+ * @of_parse_dt_fn: device specific function to parse the device tree.
+ * @priv: driver private data to be passed to of_parse_dt_fn.
+ *
+ * This routine provides unified way of getting device platform data.
+ * If the platform data exists, just return it. It's exactly same as
+ * how to get device platform data by using dev_get_platdata().
+ * If the platform data is null, checks the device node. If the device tree is
+ * is supported, then allocates the device platform data and call back to
+ * driver specific 'of_parse_dt_fn'. The caller driver should handle data
+ * manipulation inside this function.
+ *
+ * of_parse_dt_fn() has three arguments. The first is device structure.
+ * The second is the pointer of allocated device platform data.
+ * The last one is private data pointer which is used in of_parse_dt_fn().
+ *
+ * Return value is a pointer of device platform data.
+ * Caller should check IS_ERR(pdata) and return PTR_ERR(pdata).
+ */
+void *of_dev_get_platdata(struct device *dev, size_t size,
+ of_parse_dt_t of_parse_dt_fn, void *priv)
+{
+ void *pdata = dev_get_platdata(dev);
+ int err;
+
+ if (pdata)
+ return pdata;
+
+ if (!dev_of_node(dev) || !of_parse_dt_fn)
+ return NULL;
+
+ pdata = devm_kzalloc(dev, size, GFP_KERNEL);
+ if (!pdata)
+ return ERR_PTR(-ENOMEM);
+
+ err = of_parse_dt_fn(dev, pdata, priv);
+ if (err)
+ return ERR_PTR(err);
+
+ return pdata;
+}
+EXPORT_SYMBOL_GPL(of_dev_get_platdata);
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index cc7dd687..b24b3d8 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -10,6 +10,9 @@
struct device;
+/* Device specific DT parser function */
+typedef int (*of_parse_dt_t)(struct device *, void *, void *);
+
#ifdef CONFIG_OF
extern const struct of_device_id *of_match_device(
const struct of_device_id *matches, const struct device *dev);
@@ -56,6 +59,9 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
}
void of_dma_configure(struct device *dev, struct device_node *np);
+
+extern void *of_dev_get_platdata(struct device *dev, size_t size,
+ of_parse_dt_t of_parse_dt_fn, void *priv);
#else /* CONFIG_OF */
static inline int of_driver_match_device(struct device *dev,
@@ -100,6 +106,12 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
}
static inline void of_dma_configure(struct device *dev, struct device_node *np)
{}
+
+static inline void *of_dev_get_platdata(struct device *dev, size_t size,
+ of_parse_dt_t of_parse_dt_fn, void *priv)
+{
+ return dev_get_platdata(dev);
+}
#endif /* CONFIG_OF */
#endif /* _LINUX_OF_DEVICE_H */
--
1.9.1
Driver calls of_dev_get_platdata(). Error handler is added - IS_ERR() and
PTR_ERR() if an error is found.
Return type of mms114_parse_dt() is changed to integer. So return values
are modified.
Cc: Dmitry Torokhov <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Samuel Ortiz <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Milo Kim <[email protected]>
---
drivers/input/touchscreen/mms114.c | 34 ++++++++++------------------------
1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 67c0d31..555e326 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/delay.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/i2c.h>
#include <linux/i2c/mms114.h>
#include <linux/input/mt.h>
@@ -376,29 +377,19 @@ static void mms114_input_close(struct input_dev *dev)
mms114_stop(data);
}
-#ifdef CONFIG_OF
-static struct mms114_platform_data *mms114_parse_dt(struct device *dev)
+static int mms114_parse_dt(struct device *dev, void *data, void *priv)
{
- struct mms114_platform_data *pdata;
+ struct mms114_platform_data *pdata = data;
struct device_node *np = dev->of_node;
- if (!np)
- return NULL;
-
- pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
- if (!pdata) {
- dev_err(dev, "failed to allocate platform data\n");
- return NULL;
- }
-
if (of_property_read_u32(np, "x-size", &pdata->x_size)) {
dev_err(dev, "failed to get x-size property\n");
- return NULL;
+ return -EINVAL;
};
if (of_property_read_u32(np, "y-size", &pdata->y_size)) {
dev_err(dev, "failed to get y-size property\n");
- return NULL;
+ return -EINVAL;
};
of_property_read_u32(np, "contact-threshold",
@@ -411,14 +402,8 @@ static struct mms114_platform_data *mms114_parse_dt(struct device *dev)
if (of_find_property(np, "y-invert", NULL))
pdata->y_invert = true;
- return pdata;
-}
-#else
-static inline struct mms114_platform_data *mms114_parse_dt(struct device *dev)
-{
- return NULL;
+ return 0;
}
-#endif
static int mms114_probe(struct i2c_client *client,
const struct i2c_device_id *id)
@@ -428,9 +413,10 @@ static int mms114_probe(struct i2c_client *client,
struct input_dev *input_dev;
int error;
- pdata = dev_get_platdata(&client->dev);
- if (!pdata)
- pdata = mms114_parse_dt(&client->dev);
+ pdata = of_dev_get_platdata(&client->dev, sizeof(*pdata),
+ mms114_parse_dt, NULL);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
if (!pdata) {
dev_err(&client->dev, "Need platform data\n");
--
1.9.1
Platform data allocation, CONFIG_OF and condition statements are supported
in of_dev_get_platdata().
This patch shows how to use private data in each parser function.
tps65910 calls of_dev_get_platdata() with driver private data, 'chip_id'.
This data is used in tps65910_parse_dt().
'of_pmic_plat_data' is unnecessary any more. IRQ number is updated after
parsing the DT.
Cc: Dmitry Torokhov <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Samuel Ortiz <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Milo Kim <[email protected]>
---
drivers/mfd/tps65910.c | 49 ++++++++++++++++---------------------------------
1 file changed, 16 insertions(+), 33 deletions(-)
diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index 7612d89..2a068d7 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -378,7 +378,6 @@ err_sleep_init:
return ret;
}
-#ifdef CONFIG_OF
static const struct of_device_id tps65910_of_match[] = {
{ .compatible = "ti,tps65910", .data = (void *)TPS65910},
{ .compatible = "ti,tps65911", .data = (void *)TPS65911},
@@ -386,30 +385,23 @@ static const struct of_device_id tps65910_of_match[] = {
};
MODULE_DEVICE_TABLE(of, tps65910_of_match);
-static struct tps65910_board *tps65910_parse_dt(struct i2c_client *client,
- unsigned long *chip_id)
+static int tps65910_parse_dt(struct device *dev, void *data, void *priv)
{
- struct device_node *np = client->dev.of_node;
- struct tps65910_board *board_info;
+ struct device_node *np = dev->of_node;
+ struct tps65910_board *board_info = data;
+ unsigned long *chip_id = priv;
unsigned int prop;
const struct of_device_id *match;
int ret = 0;
- match = of_match_device(tps65910_of_match, &client->dev);
+ match = of_match_device(tps65910_of_match, dev);
if (!match) {
- dev_err(&client->dev, "Failed to find matching dt id\n");
- return NULL;
+ dev_err(dev, "Failed to find matching dt id\n");
+ return -EINVAL;
}
*chip_id = (unsigned long)match->data;
- board_info = devm_kzalloc(&client->dev, sizeof(*board_info),
- GFP_KERNEL);
- if (!board_info) {
- dev_err(&client->dev, "Failed to allocate pdata\n");
- return NULL;
- }
-
ret = of_property_read_u32(np, "ti,vmbch-threshold", &prop);
if (!ret)
board_info->vmbch_threshold = prop;
@@ -421,21 +413,12 @@ static struct tps65910_board *tps65910_parse_dt(struct i2c_client *client,
prop = of_property_read_bool(np, "ti,en-ck32k-xtal");
board_info->en_ck32k_xtal = prop;
- board_info->irq = client->irq;
board_info->irq_base = -1;
board_info->pm_off = of_property_read_bool(np,
"ti,system-power-controller");
- return board_info;
-}
-#else
-static inline
-struct tps65910_board *tps65910_parse_dt(struct i2c_client *client,
- unsigned long *chip_id)
-{
- return NULL;
+ return 0;
}
-#endif
static struct i2c_client *tps65910_i2c_client;
static void tps65910_power_off(void)
@@ -457,21 +440,21 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
{
struct tps65910 *tps65910;
struct tps65910_board *pmic_plat_data;
- struct tps65910_board *of_pmic_plat_data = NULL;
struct tps65910_platform_data *init_data;
unsigned long chip_id = id->driver_data;
int ret = 0;
- pmic_plat_data = dev_get_platdata(&i2c->dev);
-
- if (!pmic_plat_data && i2c->dev.of_node) {
- pmic_plat_data = tps65910_parse_dt(i2c, &chip_id);
- of_pmic_plat_data = pmic_plat_data;
- }
+ pmic_plat_data = of_dev_get_platdata(&i2c->dev,
+ sizeof(*pmic_plat_data),
+ tps65910_parse_dt, &chip_id);
+ if (IS_ERR(pmic_plat_data))
+ return PTR_ERR(pmic_plat_data);
if (!pmic_plat_data)
return -EINVAL;
+ pmic_plat_data->irq = i2c->irq;
+
init_data = devm_kzalloc(&i2c->dev, sizeof(*init_data), GFP_KERNEL);
if (init_data == NULL)
return -ENOMEM;
@@ -480,7 +463,7 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
if (tps65910 == NULL)
return -ENOMEM;
- tps65910->of_plat_data = of_pmic_plat_data;
+ tps65910->of_plat_data = pmic_plat_data;
i2c_set_clientdata(i2c, tps65910);
tps65910->dev = &i2c->dev;
tps65910->i2c_client = i2c;
--
1.9.1
'of_dev_get_platdata()' makes if-statements simple.
Cc: Dmitry Torokhov <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Samuel Ortiz <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Milo Kim <[email protected]>
---
drivers/usb/musb/ux500.c | 40 ++++++++++++++++------------------------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index 39168fe..851cf4a 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -25,6 +25,7 @@
#include <linux/err.h>
#include <linux/io.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/usb/musb-ux500.h>
@@ -200,21 +201,16 @@ static const struct musb_platform_ops ux500_ops = {
.set_vbus = ux500_musb_set_vbus,
};
-static struct musb_hdrc_platform_data *
-ux500_of_probe(struct platform_device *pdev, struct device_node *np)
+static int ux500_of_probe(struct device *dev, void *data, void *priv)
{
- struct musb_hdrc_platform_data *pdata;
+ struct musb_hdrc_platform_data *pdata = data;
const char *mode;
int strlen;
- pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
- if (!pdata)
- return NULL;
-
- mode = of_get_property(np, "dr_mode", &strlen);
+ mode = of_get_property(dev->of_node, "dr_mode", &strlen);
if (!mode) {
- dev_err(&pdev->dev, "No 'dr_mode' property found\n");
- return NULL;
+ dev_err(dev, "No 'dr_mode' property found\n");
+ return -EINVAL;
}
if (strlen > 0) {
@@ -226,31 +222,27 @@ ux500_of_probe(struct platform_device *pdev, struct device_node *np)
pdata->mode = MUSB_PERIPHERAL;
}
- return pdata;
+ return 0;
}
static int ux500_probe(struct platform_device *pdev)
{
struct resource musb_resources[2];
- struct musb_hdrc_platform_data *pdata = dev_get_platdata(&pdev->dev);
- struct device_node *np = pdev->dev.of_node;
+ struct musb_hdrc_platform_data *pdata;
struct platform_device *musb;
struct ux500_glue *glue;
struct clk *clk;
int ret = -ENOMEM;
- if (!pdata) {
- if (np) {
- pdata = ux500_of_probe(pdev, np);
- if (!pdata)
- goto err0;
+ pdata = of_dev_get_platdata(&pdev->dev, sizeof(*pdata),
+ ux500_of_probe, NULL);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
- pdev->dev.platform_data = pdata;
- } else {
- dev_err(&pdev->dev, "no pdata or device tree found\n");
- goto err0;
- }
- }
+ if (!pdata)
+ goto err0;
+
+ pdev->dev.platform_data = pdata;
glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
if (!glue)
--
1.9.1
On Fri, Aug 28, 2015 at 4:12 AM, Milo Kim <[email protected]> wrote:
> New function, 'of_dev_get_platdata()'
> - provides unified handling of getting device platform data
> - supports DT and non-DT(legacy) cases
> - removes duplicated code from each driver
> - keeps driver specific code simple in each driver
This works in cases where DT data and platform_data are aligned. In
many cases they are not. A common binding problem is people blindly
copying platform_data fields to DT properties for things that are not
h/w properties. I worry that this would encourage this behavior.
We already have a generalized method for retrieving properties
independent of DT or ACPI. There's no reason this couldn't be extended
to retrieve properties out of platform_data using the same interface.
Also, perhaps in some drivers we can remove platform_data now if all
users are converted to DT.
Rob
>
> Issues
> ------
> On loading a driver, the driver tries to get the platform data.
> * Check conditions prior to parsing the DT
> You can see various/not general way of checking the platform data.
>
> case 1) driver checks whether the platform data is null or not.
>
> foo_platform_data *pdata = dev_get_platdata(dev);
> if (!pdata) {
> pdata = foo_parse_dt();
> if (IS_ERR(pdata))
> return PTR_ERR(pdata);
> }
>
> case 2) driver checks whether 'of_node' exists or not.
>
> if (dev.of_node) {
> pdata = foo_parse_dt();
> if (IS_ERR(pdata))
> return PTR_ERR(pdata);
> }
>
> case 3) check both
>
> if (pdata) {
> copy pdata into driver private data
> } else if (dev.of_node) {
> pdata = foo_parse_dt();
> }
>
> Check conditions are very depend on the driver, but it can be unified.
> of_dev_get_platdata() provides unified handling. So the driver can reduce
> if-condition statements.
>
> * Memory allocation in each parser
> In most cases, driver allocates the platform data inside its DT parser.
>
> static struct foo_platform_data *foo_parse_dt()
> {
> allocates memory for generating platform data.
> parsing DT properties and copy them into the platform data.
> }
>
> of_dev_get_platdata() allocates the platform data internally.
> And it calls back to the driver specific parser function. It removes
> memory allocation code in each driver.
>
> * Two types of parser definition
> Many drivers implement DT parser in two cases, '#ifdef CONFIG_OF' and
> '#else'.
>
> #ifdef CONFIG_OF
> static struct foo_platform_data *foo_parse_dt()
> {
> (snip)
> }
> #else
> static struct foo_platform_data *foo_parse_dt()
> {
> return NULL;
> }
> #endif
>
> of_dev_get_platdata() supports both cases. It removes few lines of code
> in each driver.
>
> What of_dev_get_platdata() does
> -------------------------------
> if CONFIG_OF is not defined, return legacy code, 'dev_get_platdata()'.
> if platform data exists, just return it.
> if platform data is null, check the 'of node'.
> if of_node is null, then return NULL.
> Otherwise, allocates memory for platform data.
> Call back to the driver(caller) with allocated platform data and
> private data if needed.
> Then, driver will parse the DT internally and handle errors.
>
> Examples
> --------
> Following patches are examples.
>
> drivers/input/touchscreen/mms114.c
> drivers/mfd/tps65910.c
> drivers/usb/musb/ux500.c
>
> Driver list
> -----------
> of_dev_get_platdata() can be applied into files below. You may find more
> if you're interested in this. :)
>
> drivers/dma/ste_dma40.c
> drivers/gpio/gpio-rcar.c
> drivers/gpu/drm/exynos/exynos_dp_core.c
> drivers/gpu/drm/exynos/exynos_hdmi.c
> drivers/hwmon/ntc_thermistor.c
> drivers/i2c/busses/i2c-s3c2410.c
> drivers/iio/frequency/adf4350.c
> drivers/input/keyboard/matrix_keypad.c
> drivers/input/keyboard/samsung-keypad.c
> drivers/input/misc/drv260x.c
> drivers/input/misc/regulator-haptic.c
> drivers/input/misc/rotary_encoder.c
> drivers/input/touchscreen/atmel_mxt_ts.c
> drivers/input/touchscreen/auo-pixcir-ts.c
> drivers/input/touchscreen/bu21013_ts.c
> drivers/input/touchscreen/mms114.c
> drivers/input/touchscreen/pixcir_i2c_ts.c
> drivers/input/touchscreen/zforce_ts.c
> drivers/leds/leds-lp5521.c
> drivers/leds/leds-lp5523.c
> drivers/leds/leds-lp5562.c
> drivers/leds/leds-lp55xx-common.c
> drivers/leds/leds-lp8501.c
> drivers/leds/leds-mc13783.c
> drivers/leds/leds-pca963x.c
> drivers/leds/leds-tca6507.c
> drivers/mfd/max8997.c
> drivers/mfd/max8998.c
> drivers/mfd/sec-core.c
> drivers/mfd/tps6586x.c
> drivers/mfd/tps65910.c
> drivers/misc/lis3lv02d/lis3lv02d.c
> drivers/mmc/host/davinci_mmc.c
> drivers/mmc/host/dw_mmc.c
> drivers/mmc/host/sdhci-s3c.c
> drivers/mtd/devices/spear_smi.c
> drivers/mtd/nand/fsmc_nand.c
> drivers/mtd/nand/lpc32xx_mlc.c
> drivers/mtd/nand/lpc32xx_slc.c
> drivers/mtd/nand/sh_flctl.c
> drivers/net/can/sja1000/sja1000_platform.c
> drivers/net/ethernet/davicom/dm9000.c
> drivers/net/ethernet/marvell/mv643xx_eth.c
> drivers/net/ethernet/renesas/sh_eth.c
> drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> drivers/power/bq24735-charger.c
> drivers/power/gpio-charger.c
> drivers/power/sbs-battery.c
> drivers/power/tps65090-charger.c
> drivers/power/twl4030_charger.c
> drivers/pwm/pwm-lp3943.c
> drivers/pwm/pwm-samsung.c
> drivers/spi/spi-sh-msiof.c
> drivers/spi/spi/spi-s3c64xx.c
> drivers/thermal/db8500_thermal.c
> drivers/usb/misc/usb3503.c
> drivers/usb/musb/omap2430.c
> drivers/usb/musb/ux500.c
> drivers/usb/renesas_usbhs/common.c
> drivers/video/fbdev/simplefb.c
> drivers/video/backlight/lp855x_bl.c
> drivers/video/backlight/pwm_bl.c
> drivers/video/backlight/sky81452-backlight.c
> drivers/video/backlight/tps65217_bl.c
>
> This is the RFC, so I would like to get some feedback prior to patching all
> drivers. Any comments are welcome.
>
> Cc: Dmitry Torokhov <[email protected]>
> Cc: Felipe Balbi <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Lee Jones <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> Cc: Tony Lindgren <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
>
> Milo Kim (4):
> of: introduce of_dev_get_platdata()
> input: touchscree: mms114: use of_dev_get_platdata()
> mfd: tps65910: use of_dev_get_platdata()
> usb: musb: use of_dev_get_platdata()
>
> drivers/input/touchscreen/mms114.c | 34 ++++++++------------------
> drivers/mfd/tps65910.c | 49 +++++++++++++-------------------------
> drivers/of/device.c | 46 +++++++++++++++++++++++++++++++++++
> drivers/usb/musb/ux500.c | 40 +++++++++++++------------------
> include/linux/of_device.h | 12 ++++++++++
> 5 files changed, 100 insertions(+), 81 deletions(-)
>
> --
> 1.9.1
>
Hi Rob,
On 8/28/2015 9:59 PM, Rob Herring wrote:
> On Fri, Aug 28, 2015 at 4:12 AM, Milo Kim<[email protected]> wrote:
>> >New function, 'of_dev_get_platdata()'
>> > - provides unified handling of getting device platform data
>> > - supports DT and non-DT(legacy) cases
>> > - removes duplicated code from each driver
>> > - keeps driver specific code simple in each driver
> This works in cases where DT data and platform_data are aligned. In
> many cases they are not. A common binding problem is people blindly
> copying platform_data fields to DT properties for things that are not
> h/w properties. I worry that this would encourage this behavior.
OK, got your point. Thanks.
>
> We already have a generalized method for retrieving properties
> independent of DT or ACPI. There's no reason this couldn't be extended
> to retrieve properties out of platform_data using the same interface.
>
> Also, perhaps in some drivers we can remove platform_data now if all
> users are converted to DT.
This work requires two steps.
1) remove platform data configuration from board-*.c
2) if all host/platform support only the DT, then move device platform
data structure in a header to each driver space.
However, I'm concerning about some use case.
After this cleanup, drivers may not work properly in some platforms
which do not support the DT. There is no way to configure HW properties.
Drivers should run on any kind of hosts(processors) so I'm thinking
about the compatibility/dependency issue. Am I missing something?
Milo