| Message ID | 1358999112-31192-6-git-send-email-rachna@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 24 jan. 2013, om 04:45 heeft "Patil, Rachna" <rachna@ti.com> het volgende geschreven: > From: "Patil, Rachna" <rachna@ti.com> > > Make changes to add DT support in the MFD core driver. > > Signed-off-by: Patil, Rachna <rachna@ti.com> > --- > Changes in v4: > Non-standard properties prefixed with vendor name. > > drivers/mfd/ti_am335x_tscadc.c | 28 +++++++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c > index e9f3fb5..87b446b 100644 > --- a/drivers/mfd/ti_am335x_tscadc.c > +++ b/drivers/mfd/ti_am335x_tscadc.c > @@ -22,6 +22,8 @@ > #include <linux/regmap.h> > #include <linux/mfd/core.h> > #include <linux/pm_runtime.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > > #include <linux/mfd/ti_am335x_tscadc.h> > #include <linux/input/ti_am335x_tsc.h> > @@ -64,20 +66,31 @@ static int ti_tscadc_probe(struct platform_device *pdev) > struct resource *res; > struct clk *clk; > struct mfd_tscadc_board *pdata = pdev->dev.platform_data; > + struct device_node *node = pdev->dev.of_node; > struct mfd_cell *cell; > int err, ctrl; > int clk_value, clock_rate; > - int tsc_wires, adc_channels = 0, total_channels; > + int tsc_wires = 0, adc_channels = 0, total_channels; > > - if (!pdata) { > + if (!pdata && !pdev->dev.of_node) { > dev_err(&pdev->dev, "Could not find platform data\n"); > return -EINVAL; > } > > - if (pdata->adc_init) > - adc_channels = pdata->adc_init->adc_channels; > + if (pdev->dev.platform_data) { > + if (pdata->tsc_init) > + tsc_wires = pdata->tsc_init->wires; > + > + if (pdata->adc_init) > + adc_channels = pdata->adc_init->adc_channels; > + } else { > + node = of_find_node_by_name(pdev->dev.of_node, "tsc"); > + of_property_read_u32(node, "ti,wires", &tsc_wires); > + > + node = of_find_node_by_name(pdev->dev.of_node, "adc"); > + of_property_read_u32(node, "ti,adc-channels", &adc_channels); > + } Since AM335x is DT only, why is there a platform data codepath and why is it the first branch it tries? And I guess the next question is related to the first: why doesn't it work when used with DT? When I copy over the nodes from the evm.dts to my board I get "tsc tsc: Missing platform data" in dmesg. What are the chances this driver will work when applied on top of 3.8-rcX? Has it even been tested with that? Has it been tested at all? -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 30, 2013 at 16:10:09, Koen Kooi wrote: > > Op 24 jan. 2013, om 04:45 heeft "Patil, Rachna" <rachna@ti.com> het volgende geschreven: > > > From: "Patil, Rachna" <rachna@ti.com> > > > > Make changes to add DT support in the MFD core driver. > > > > Signed-off-by: Patil, Rachna <rachna@ti.com> > > --- > > Changes in v4: > > Non-standard properties prefixed with vendor name. > > > > drivers/mfd/ti_am335x_tscadc.c | 28 +++++++++++++++++++++++----- > > 1 file changed, 23 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/mfd/ti_am335x_tscadc.c > > b/drivers/mfd/ti_am335x_tscadc.c index e9f3fb5..87b446b 100644 > > --- a/drivers/mfd/ti_am335x_tscadc.c > > +++ b/drivers/mfd/ti_am335x_tscadc.c > > @@ -22,6 +22,8 @@ > > #include <linux/regmap.h> > > #include <linux/mfd/core.h> > > #include <linux/pm_runtime.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > > > #include <linux/mfd/ti_am335x_tscadc.h> #include > > <linux/input/ti_am335x_tsc.h> > > @@ -64,20 +66,31 @@ static int ti_tscadc_probe(struct platform_device *pdev) > > struct resource *res; > > struct clk *clk; > > struct mfd_tscadc_board *pdata = pdev->dev.platform_data; > > + struct device_node *node = pdev->dev.of_node; > > struct mfd_cell *cell; > > int err, ctrl; > > int clk_value, clock_rate; > > - int tsc_wires, adc_channels = 0, total_channels; > > + int tsc_wires = 0, adc_channels = 0, total_channels; > > > > - if (!pdata) { > > + if (!pdata && !pdev->dev.of_node) { > > dev_err(&pdev->dev, "Could not find platform data\n"); > > return -EINVAL; > > } > > > > - if (pdata->adc_init) > > - adc_channels = pdata->adc_init->adc_channels; > > + if (pdev->dev.platform_data) { > > + if (pdata->tsc_init) > > + tsc_wires = pdata->tsc_init->wires; > > + > > + if (pdata->adc_init) > > + adc_channels = pdata->adc_init->adc_channels; > > + } else { > > + node = of_find_node_by_name(pdev->dev.of_node, "tsc"); > > + of_property_read_u32(node, "ti,wires", &tsc_wires); > > + > > + node = of_find_node_by_name(pdev->dev.of_node, "adc"); > > + of_property_read_u32(node, "ti,adc-channels", &adc_channels); > > + } > > Since AM335x is DT only, why is there a platform data codepath and why is it the first branch it tries? And I guess the next question is related to the first: why doesn't it work when used with DT? When I copy over the nodes from the evm.dts to my board I get "tsc tsc: Missing platform data" in dmesg. This IP came up 1st on AM335x, but it is not platform dependent. The driver can be used on other platforms where-in DT is not supported. According to the maintainers platform data takes precedence over DT. Hence the order. I do not see "Missing platform data" error msg in the latest driver. I am not able to trace from where this got populated. > > What are the chances this driver will work when applied on top of 3.8-rcX? Has it even been tested with that? Has it been tested at all? This driver has been tested on top of v3.8-rc3 on a AM335x EVM. Regards, Rachna -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 31, 2013 at 09:41:11, Patil, Rachna wrote: > On Wed, Jan 30, 2013 at 16:10:09, Koen Kooi wrote: > > > > Op 24 jan. 2013, om 04:45 heeft "Patil, Rachna" <rachna@ti.com> het volgende geschreven: > > > > > From: "Patil, Rachna" <rachna@ti.com> > > > > > > Make changes to add DT support in the MFD core driver. > > > > > > Signed-off-by: Patil, Rachna <rachna@ti.com> > > > --- > > > Changes in v4: > > > Non-standard properties prefixed with vendor name. > > > > > > drivers/mfd/ti_am335x_tscadc.c | 28 +++++++++++++++++++++++----- > > > 1 file changed, 23 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/mfd/ti_am335x_tscadc.c > > > b/drivers/mfd/ti_am335x_tscadc.c index e9f3fb5..87b446b 100644 > > > --- a/drivers/mfd/ti_am335x_tscadc.c > > > +++ b/drivers/mfd/ti_am335x_tscadc.c > > > @@ -22,6 +22,8 @@ > > > #include <linux/regmap.h> > > > #include <linux/mfd/core.h> > > > #include <linux/pm_runtime.h> > > > +#include <linux/of.h> > > > +#include <linux/of_device.h> > > > > > > #include <linux/mfd/ti_am335x_tscadc.h> #include > > > <linux/input/ti_am335x_tsc.h> > > > @@ -64,20 +66,31 @@ static int ti_tscadc_probe(struct platform_device *pdev) > > > struct resource *res; > > > struct clk *clk; > > > struct mfd_tscadc_board *pdata = pdev->dev.platform_data; > > > + struct device_node *node = pdev->dev.of_node; > > > struct mfd_cell *cell; > > > int err, ctrl; > > > int clk_value, clock_rate; > > > - int tsc_wires, adc_channels = 0, total_channels; > > > + int tsc_wires = 0, adc_channels = 0, total_channels; > > > > > > - if (!pdata) { > > > + if (!pdata && !pdev->dev.of_node) { > > > dev_err(&pdev->dev, "Could not find platform data\n"); > > > return -EINVAL; > > > } > > > > > > - if (pdata->adc_init) > > > - adc_channels = pdata->adc_init->adc_channels; > > > + if (pdev->dev.platform_data) { > > > + if (pdata->tsc_init) > > > + tsc_wires = pdata->tsc_init->wires; > > > + > > > + if (pdata->adc_init) > > > + adc_channels = pdata->adc_init->adc_channels; > > > + } else { > > > + node = of_find_node_by_name(pdev->dev.of_node, "tsc"); > > > + of_property_read_u32(node, "ti,wires", &tsc_wires); > > > + > > > + node = of_find_node_by_name(pdev->dev.of_node, "adc"); > > > + of_property_read_u32(node, "ti,adc-channels", &adc_channels); > > > + } > > > > Since AM335x is DT only, why is there a platform data codepath and why is it the first branch it tries? And I guess the next question is related to the first: why doesn't it work when used with DT? When I copy over the nodes from the evm.dts to my board I get "tsc tsc: Missing platform data" in dmesg. > > This IP came up 1st on AM335x, but it is not platform dependent. The driver can be used on other platforms where-in DT is not supported. > According to the maintainers platform data takes precedence over DT. Hence the order. > Rachana, I see no point adding support for platform_data when you know that none of older platforms are going to use this driver and all future platforms _must_ follow device-tree model. So I agree that you should remove board file dependency from the driver. > I do not see "Missing platform data" error msg in the latest driver. I am not able to trace from where this got populated. > Can you share the branch which you have tested? Thanks, Vaibhav > > > > What are the chances this driver will work when applied on top of 3.8-rcX? Has it even been tested with that? Has it been tested at all? > > This driver has been tested on top of v3.8-rc3 on a AM335x EVM. > > Regards, > Rachna > > _______________________________________________ > devicetree-discuss mailing list > devicetree-discuss@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Vaibhav, <SNIP> > > > > > > > > - if (!pdata) { > > > > + if (!pdata && !pdev->dev.of_node) { > > > > dev_err(&pdev->dev, "Could not find platform data\n"); > > > > return -EINVAL; > > > > } > > > > > > > > - if (pdata->adc_init) > > > > - adc_channels = pdata->adc_init->adc_channels; > > > > + if (pdev->dev.platform_data) { > > > > + if (pdata->tsc_init) > > > > + tsc_wires = pdata->tsc_init->wires; > > > > + > > > > + if (pdata->adc_init) > > > > + adc_channels = pdata->adc_init->adc_channels; > > > > + } else { > > > > + node = of_find_node_by_name(pdev->dev.of_node, "tsc"); > > > > + of_property_read_u32(node, "ti,wires", &tsc_wires); > > > > + > > > > + node = of_find_node_by_name(pdev->dev.of_node, "adc"); > > > > + of_property_read_u32(node, "ti,adc-channels", &adc_channels); > > > > + } > > > > > > Since AM335x is DT only, why is there a platform data codepath and why is it the first branch it tries? And I guess the next question is related to the first: why doesn't it work when used with DT? When I copy over the nodes from the evm.dts to my board I get "tsc tsc: Missing platform data" in dmesg. > > > > This IP came up 1st on AM335x, but it is not platform dependent. The driver can be used on other platforms where-in DT is not supported. > > According to the maintainers platform data takes precedence over DT. Hence the order. > > > > Rachana, > > I see no point adding support for platform_data when you know that none of older platforms are going to use this driver and all future platforms _must_ follow device-tree model. > > So I agree that you should remove board file dependency from the driver. Ok. I will remove support for platform data in the next version of patches. > > > > I do not see "Missing platform data" error msg in the latest driver. I am not able to trace from where this got populated. > > > > Can you share the branch which you have tested? https://github.com/patilrachna/linux/tree/v3.8_rc3_MFD_TSCADC_DT-v2 Regards, Rachna -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Koen, > <SNIP> > > > > Since AM335x is DT only, why is there a platform data codepath and why is it the first branch it tries? And I guess the next question is related to the first: why doesn't it work when used with DT? When I copy over the nodes from the evm.dts to my board I get "tsc tsc: Missing platform data" in dmesg. > > > > > > This IP came up 1st on AM335x, but it is not platform dependent. The driver can be used on other platforms where-in DT is not supported. > > > According to the maintainers platform data takes precedence over DT. Hence the order. > > > > > > > Rachana, > > > > I see no point adding support for platform_data when you know that none of older platforms are going to use this driver and all future platforms _must_ follow device-tree model. > > > > So I agree that you should remove board file dependency from the driver. > > Ok. I will remove support for platform data in the next version of patches. > > > > > > > > I do not see "Missing platform data" error msg in the latest driver. I am not able to trace from where this got populated. > > > > > > > Can you share the branch which you have tested? > > https://github.com/patilrachna/linux/tree/v3.8_rc3_MFD_TSCADC_DT-v2 Did you get a chance to test this branch? And can you also share your branch, on which you observed the issue. Regards, Rachna -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-02-06 at 11:17 +0000, Patil, Rachna wrote: > > > Can you share the branch which you have tested? > > > > https://github.com/patilrachna/linux/tree/v3.8_rc3_MFD_TSCADC_DT-v2 > > Did you get a chance to test this branch? > And can you also share your branch, on which you observed the issue. Rachana, I have not seen a new version of this patch since January. Are you still working on the TSCADC? If not, where can I find the most recent version? Regards, Jan
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c index e9f3fb5..87b446b 100644 --- a/drivers/mfd/ti_am335x_tscadc.c +++ b/drivers/mfd/ti_am335x_tscadc.c @@ -22,6 +22,8 @@ #include <linux/regmap.h> #include <linux/mfd/core.h> #include <linux/pm_runtime.h> +#include <linux/of.h> +#include <linux/of_device.h> #include <linux/mfd/ti_am335x_tscadc.h> #include <linux/input/ti_am335x_tsc.h> @@ -64,20 +66,31 @@ static int ti_tscadc_probe(struct platform_device *pdev) struct resource *res; struct clk *clk; struct mfd_tscadc_board *pdata = pdev->dev.platform_data; + struct device_node *node = pdev->dev.of_node; struct mfd_cell *cell; int err, ctrl; int clk_value, clock_rate; - int tsc_wires, adc_channels = 0, total_channels; + int tsc_wires = 0, adc_channels = 0, total_channels; - if (!pdata) { + if (!pdata && !pdev->dev.of_node) { dev_err(&pdev->dev, "Could not find platform data\n"); return -EINVAL; } - if (pdata->adc_init) - adc_channels = pdata->adc_init->adc_channels; + if (pdev->dev.platform_data) { + if (pdata->tsc_init) + tsc_wires = pdata->tsc_init->wires; + + if (pdata->adc_init) + adc_channels = pdata->adc_init->adc_channels; + } else { + node = of_find_node_by_name(pdev->dev.of_node, "tsc"); + of_property_read_u32(node, "ti,wires", &tsc_wires); + + node = of_find_node_by_name(pdev->dev.of_node, "adc"); + of_property_read_u32(node, "ti,adc-channels", &adc_channels); + } - tsc_wires = pdata->tsc_init->wires; total_channels = tsc_wires + adc_channels; if (total_channels > 8) { @@ -256,11 +269,16 @@ static const struct dev_pm_ops tscadc_pm_ops = { #define TSCADC_PM_OPS NULL #endif +static const struct of_device_id ti_tscadc_dt_ids[] = { + { .compatible = "ti,ti-tscadc", }, +}; + static struct platform_driver ti_tscadc_driver = { .driver = { .name = "ti_tscadc", .owner = THIS_MODULE, .pm = TSCADC_PM_OPS, + .of_match_table = of_match_ptr(ti_tscadc_dt_ids), }, .probe = ti_tscadc_probe, .remove = ti_tscadc_remove,