| Message ID | 20200504092611.9798-27-laurent.pinchart@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series | Drivers for the BCM283x CSI-2/CCP2 receiver and ISP | expand |
Hi Nicolas, Phil,
I'm in the process of sending a v2 of this series, trying to
reduce the patch count by only picking what's required to support
the ISP.
This patch and [30/34] puzzles me a bit, so I'll ask a few questions.
On Mon, May 04, 2020 at 06:54:32PM +0200, Nicolas Saenz Julienne wrote:
> On Mon, 2020-05-04 at 12:26 +0300, Laurent Pinchart wrote:
> > From: Dave Stevenson <dave.stevenson@raspberrypi.org>
> >
> > The VCHIQ driver now loads the audio, camera, codec, and vc-sm
> > drivers as platform drivers. However they were not being given
> > the correct DMA configuration.
> >
> > Call of_dma_configure with the parent (VCHIQ) parameters to be
> > inherited by the child.
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> > .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index c5c7af28c1c8..15ccd624aaab 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -2733,6 +2733,12 @@ vchiq_register_child(struct platform_device *pdev,
> > const char *name)
> > child = NULL;
> > }
> >
> > + /*
> > + * We want the dma-ranges etc to be copied from the parent VCHIQ device
> > + * to be passed on to the children too.
> > + */
> > + of_dma_configure(&new_dev->dev, pdev->dev.of_node, true);
>
> I think you could use struct platform_device_info's of_node_reused. See patch
> 908f6fc3a1405 ('usb: musb: sunxi: propagate devicetree node to glue pdev') for
> an example. AFAIK of_dma_configure() is only to be used in bus code, and there
> has been a huge effort in the past to make sure it says as such. With a proper
> fwnode set of_dma_configure() will be called during the device's probe.
I just tried giving to the children devices the parent's fwnode and set
of_node_reused as done in 908f6fc3a1405 to avoid usage of
of_dma_configure, and now I have the parent being probed 4 times, one per
each children :) I guess that was not the expected result...
What is in your opinions the best course of actions for this and patch
[30/34] ? Can we live with 'of_dma_configure()' ? [1]
I am assuming currently the vchiq children are manually registered as
there are no entries in the DTS for them, so I'm kind of missing the
point of having [30/34] in. What am I missing ?
Thanks
j
[1] There are a few things that are not clear to me though, possible due
to my rather poor understanding of this core parts. In particular, as
the dma-ranges are assigned to the parent 'soc' node, and I assume
propagated to the children such as the 'vchiq' node, isnt' setting the
'parent' field of each newly created platform device enough to have
the property propagated ?
>
> Regards,
> Nicolas
>
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index c5c7af28c1c8..15ccd624aaab 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2733,6 +2733,12 @@ vchiq_register_child(struct platform_device *pdev, const char *name) child = NULL; } + /* + * We want the dma-ranges etc to be copied from the parent VCHIQ device + * to be passed on to the children too. + */ + of_dma_configure(&new_dev->dev, pdev->dev.of_node, true); + return child; }