| Message ID | 20170508113303.27521-14-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/05/17 14:32, Laurent Pinchart wrote: > The DSS internal features are derived from the platform device > compatible string which is available at probe time. Don't delay > initialization until bind time. This prepares for the merge of the two > DSS features structures that will be needed before the DSS is bound. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/omapdrm/dss/dss.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c > index 7546ba8ab955..4fdb77dd90b8 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dss.c > +++ b/drivers/gpu/drm/omapdrm/dss/dss.c > @@ -1183,23 +1183,10 @@ static const struct soc_device_attribute dss_soc_devices[] = { > static int dss_bind(struct device *dev) > { > struct platform_device *pdev = to_platform_device(dev); > - const struct soc_device_attribute *soc; > struct resource *dss_mem; > u32 rev; > int r; > > - dss.pdev = pdev; > - > - /* > - * The various OMAP3-based SoCs can be told apart from the compatible > - * string, use SoC device matching. > - */ > - soc = soc_device_match(dss_soc_devices); > - if (soc) > - dss.feat = soc->data; > - else > - dss.feat = of_match_device(dss_of_match, &pdev->dev)->data; > - > dss_mem = platform_get_resource(dss.pdev, IORESOURCE_MEM, 0); > dss.base = devm_ioremap_resource(&pdev->dev, dss_mem); > if (!dss.base) > @@ -1331,9 +1318,22 @@ static int dss_add_child_component(struct device *dev, void *data) > > static int dss_probe(struct platform_device *pdev) > { > + const struct soc_device_attribute *soc; > struct component_match *match = NULL; > int r; > > + dss.pdev = pdev; > + > + /* > + * The various OMAP3-based SoCs can be told apart from the compatible > + * string, use SoC device matching. > + */ > + soc = soc_device_match(dss_soc_devices); > + if (soc) > + dss.feat = soc->data; > + else > + dss.feat = of_match_device(dss_of_match, &pdev->dev)->data; > + This has the problem that we should remove the static 'dss' variable, and allocate memory for each device. We can have more than one DSS in the future. Also, I haven't looked at the following patches yet, but dss_features was a failed attempt to manage the dss features. Since then, we've slowly been moving the bits and pieces from dss_features to each individual driver. So if this is needed for dss_features, we should instead move everything from dss_features and remove that altogether. Tomi
Hi Tomi, On Tuesday 09 May 2017 12:48:57 Tomi Valkeinen wrote: > On 08/05/17 14:32, Laurent Pinchart wrote: > > The DSS internal features are derived from the platform device > > compatible string which is available at probe time. Don't delay > > initialization until bind time. This prepares for the merge of the two > > DSS features structures that will be needed before the DSS is bound. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > > > drivers/gpu/drm/omapdrm/dss/dss.c | 26 +++++++++++++------------- > > 1 file changed, 13 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c > > b/drivers/gpu/drm/omapdrm/dss/dss.c index 7546ba8ab955..4fdb77dd90b8 > > 100644 > > --- a/drivers/gpu/drm/omapdrm/dss/dss.c > > +++ b/drivers/gpu/drm/omapdrm/dss/dss.c > > @@ -1183,23 +1183,10 @@ static const struct soc_device_attribute > > dss_soc_devices[] = {> > > static int dss_bind(struct device *dev) > > { > > > > struct platform_device *pdev = to_platform_device(dev); > > > > - const struct soc_device_attribute *soc; > > > > struct resource *dss_mem; > > u32 rev; > > int r; > > > > - dss.pdev = pdev; > > - > > - /* > > - * The various OMAP3-based SoCs can be told apart from the compatible > > - * string, use SoC device matching. > > - */ > > - soc = soc_device_match(dss_soc_devices); > > - if (soc) > > - dss.feat = soc->data; > > - else > > - dss.feat = of_match_device(dss_of_match, &pdev->dev)->data; > > - > > > > dss_mem = platform_get_resource(dss.pdev, IORESOURCE_MEM, 0); > > dss.base = devm_ioremap_resource(&pdev->dev, dss_mem); > > if (!dss.base) > > > > @@ -1331,9 +1318,22 @@ static int dss_add_child_component(struct device > > *dev, void *data)> > > static int dss_probe(struct platform_device *pdev) > > { > > > > + const struct soc_device_attribute *soc; > > > > struct component_match *match = NULL; > > int r; > > > > + dss.pdev = pdev; > > + > > + /* > > + * The various OMAP3-based SoCs can be told apart from the compatible > > + * string, use SoC device matching. > > + */ > > + soc = soc_device_match(dss_soc_devices); > > + if (soc) > > + dss.feat = soc->data; > > + else > > + dss.feat = of_match_device(dss_of_match, &pdev->dev)->data; > > + > > This has the problem that we should remove the static 'dss' variable, > and allocate memory for each device. We can have more than one DSS in > the future. Sure, and that's on my to-do list, but I didn't want to make the patch series bigger than it is already. > Also, I haven't looked at the following patches yet, but dss_features > was a failed attempt to manage the dss features. Since then, we've > slowly been moving the bits and pieces from dss_features to each > individual driver. > > So if this is needed for dss_features, we should instead move everything > from dss_features and remove that altogether. I agree with you, but again, I've tried to keep the existing architecture here, we can certainly rework it as a next step.
diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c b/drivers/gpu/drm/omapdrm/dss/dss.c index 7546ba8ab955..4fdb77dd90b8 100644 --- a/drivers/gpu/drm/omapdrm/dss/dss.c +++ b/drivers/gpu/drm/omapdrm/dss/dss.c @@ -1183,23 +1183,10 @@ static const struct soc_device_attribute dss_soc_devices[] = { static int dss_bind(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); - const struct soc_device_attribute *soc; struct resource *dss_mem; u32 rev; int r; - dss.pdev = pdev; - - /* - * The various OMAP3-based SoCs can be told apart from the compatible - * string, use SoC device matching. - */ - soc = soc_device_match(dss_soc_devices); - if (soc) - dss.feat = soc->data; - else - dss.feat = of_match_device(dss_of_match, &pdev->dev)->data; - dss_mem = platform_get_resource(dss.pdev, IORESOURCE_MEM, 0); dss.base = devm_ioremap_resource(&pdev->dev, dss_mem); if (!dss.base) @@ -1331,9 +1318,22 @@ static int dss_add_child_component(struct device *dev, void *data) static int dss_probe(struct platform_device *pdev) { + const struct soc_device_attribute *soc; struct component_match *match = NULL; int r; + dss.pdev = pdev; + + /* + * The various OMAP3-based SoCs can be told apart from the compatible + * string, use SoC device matching. + */ + soc = soc_device_match(dss_soc_devices); + if (soc) + dss.feat = soc->data; + else + dss.feat = of_match_device(dss_of_match, &pdev->dev)->data; + /* add all the child devices as components */ device_for_each_child(&pdev->dev, &match, dss_add_child_component);
The DSS internal features are derived from the platform device compatible string which is available at probe time. Don't delay initialization until bind time. This prepares for the merge of the two DSS features structures that will be needed before the DSS is bound. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/gpu/drm/omapdrm/dss/dss.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)