Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi! > > > > > Making the sub-device bus configuration a pointer should be in a > > > > > separate > > > > > patch. It makes sense since the entire configuration is not valid for > > > > > all > > > > > sub-devices attached to the ISP anymore. I think it originally was a > > > > > separate patch, but they probably have been merged at some point. I > > > > > can't > > > > > find it right now anyway. > > > > > > > > Something like this? > > > > > > > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > > > > Author: Pavel> > > > Date: Wed Mar 1 13:27:56 2017 +0100 > > > > > > > > Turn bus in struct isp_async_subdev into pointer; some of our > > > > subdevs > > > > (flash, focus) will not need bus configuration. > > > > > > > > Signed-off-by: Pavel Machek > > > > > > I applied this to the ccp2 branch with an improved patch > > > description. > > > > Thanks! > > > > [But the important part is to get subdevices to work on ccp2 based > > branch, and it still fails to work at all if I attempt to enable > > them. I'd like to understand why...] > > Did you add the flash / lens to the async list? The patches currently in the > ccp branch do not include that --- it should be in parsing the flash / > lens-focus properties in omap3isp device's node. I retried, and it fails different way than I assumed. I might be able to debug this one as sensor (and mplayer) still works. Best regards, Pavel -- This is what subdevs support should look like, I guess; but I don't know fwnode stuff well enough. diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index c80397a..36bd359 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -2166,6 +2166,8 @@ static int isp_fwnodes_parse(struct device *dev, struct v4l2_async_notifier *notifier) { struct fwnode_handle *fwn = NULL; + struct device_node *node; + int flash = 0; notifier->subdevs = devm_kcalloc( dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL); @@ -2199,6 +2201,42 @@ static int isp_fwnodes_parse(struct device *dev, notifier->num_subdevs++; } + printk("Going through camera-flashes\n"); + while (notifier->num_subdevs < ISP_MAX_SUBDEVS) { + /* FIXME: fwnode_graph_get_remote_endpoint() + (fwn = fwnode_graph_get_next_endpoint(device_fwnode_handle(dev), fwn, )) */ + struct isp_async_subdev *isd; + + node = of_parse_phandle(dev->of_node, "ti,camera-flashes", flash++); + flash++; + if (!node) + break; + + printk("Having subdevice: %p\n", node); + +#if 1 + isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL); + if (!isd) + goto error; + + notifier->subdevs[notifier->num_subdevs] = >asd; + + + isd->asd.match.of.node = node; + if (!isd->asd.match.of.node) { + dev_warn(dev, "bad remote port parent\n"); + goto error; + } + + isd->asd.match_type = V4L2_ASYNC_MATCH_OF; + notifier->num_subdevs++; +#endif + } + + + if (notifier->num_subdevs == ISP_MAX_SUBDEVS) { + printk("isp: Maybe too many devices?\n"); + } return notifier->num_subdevs; error: -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi! > > > > > Making the sub-device bus configuration a pointer should be in a > > > > > separate > > > > > patch. It makes sense since the entire configuration is not valid for > > > > > all > > > > > sub-devices attached to the ISP anymore. I think it originally was a > > > > > separate patch, but they probably have been merged at some point. I > > > > > can't > > > > > find it right now anyway. > > > > > > > > Something like this? > > > > > > > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > > > > Author: Pavel > > > > Date: Wed Mar 1 13:27:56 2017 +0100 > > > > > > > > Turn bus in struct isp_async_subdev into pointer; some of our > > > > subdevs > > > > (flash, focus) will not need bus configuration. > > > > > > > > Signed-off-by: Pavel Machek > > > > > > I applied this to the ccp2 branch with an improved patch > > > description. > > > > Thanks! > > > > [But the important part is to get subdevices to work on ccp2 based > > branch, and it still fails to work at all if I attempt to enable > > them. I'd like to understand why...] > > Did you add the flash / lens to the async list? The patches currently in the > ccp branch do not include that --- it should be in parsing the flash / > lens-focus properties in omap3isp device's node. I retried, and it fails different way than I assumed. I might be able to debug this one as sensor (and mplayer) still works. Best regards, Pavel -- This is what subdevs support should look like, I guess; but I don't know fwnode stuff well enough. diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index c80397a..36bd359 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -2166,6 +2166,8 @@ static int isp_fwnodes_parse(struct device *dev, struct v4l2_async_notifier *notifier) { struct fwnode_handle *fwn = NULL; + struct device_node *node; + int flash = 0; notifier->subdevs = devm_kcalloc( dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL); @@ -2199,6 +2201,42 @@ static int isp_fwnodes_parse(struct device *dev, notifier->num_subdevs++; } + printk("Going through camera-flashes\n"); + while (notifier->num_subdevs < ISP_MAX_SUBDEVS) { + /* FIXME: fwnode_graph_get_remote_endpoint() + (fwn = fwnode_graph_get_next_endpoint(device_fwnode_handle(dev), fwn, )) */ + struct isp_async_subdev *isd; + + node = of_parse_phandle(dev->of_node, "ti,camera-flashes", flash++); + flash++; + if (!node) + break; + + printk("Having subdevice: %p\n", node); + +#if 1 + isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL); + if (!isd) + goto error; + + notifier->subdevs[notifier->num_subdevs] = >asd; + + + isd->asd.match.of.node = node; + if (!isd->asd.match.of.node) { + dev_warn(dev, "bad remote port parent\n"); + goto error; + } + + isd->asd.match_type = V4L2_ASYNC_MATCH_OF; + notifier->num_subdevs++; +#endif + } + + + if (notifier->num_subdevs == ISP_MAX_SUBDEVS) { + printk("isp: Maybe too many devices?\n"); + } return notifier->num_subdevs; error: -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi! > > > static int isp_fwnode_parse(struct device *dev, struct fwnode_handle > > > *fwn, > > > > > > struct isp_async_subdev *isd) > > > > > > { > > > > > > - struct isp_bus_cfg *buscfg = >bus; > > > + struct isp_bus_cfg *buscfg; > > > > > > struct v4l2_fwnode_endpoint vfwn; > > > unsigned int i; > > > int ret; > > > bool csi1 = false; > > > > > > + buscfg = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL); > > Given that you recently get rid of devm_kzalloc() in the driver, let's not > introduce a new one here. What is wrong with devm_kzalloc()? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi! > > > static int isp_fwnode_parse(struct device *dev, struct fwnode_handle > > > *fwn, > > > > > > struct isp_async_subdev *isd) > > > > > > { > > > > > > - struct isp_bus_cfg *buscfg = >bus; > > > + struct isp_bus_cfg *buscfg; > > > > > > struct v4l2_fwnode_endpoint vfwn; > > > unsigned int i; > > > int ret; > > > bool csi1 = false; > > > > > > + buscfg = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL); > > Given that you recently get rid of devm_kzalloc() in the driver, let's not > introduce a new one here. What is wrong with devm_kzalloc()? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi Laurent, On Thu, Mar 02, 2017 at 08:39:51PM +0200, Laurent Pinchart wrote: > Hi Sakari, > > On Thursday 02 Mar 2017 16:16:17 Sakari Ailus wrote: > > On Thu, Mar 02, 2017 at 10:07:27AM +0100, Pavel Machek wrote: > > > Hi! > > > > > > > Making the sub-device bus configuration a pointer should be in a > > > > separate patch. It makes sense since the entire configuration is not > > > > valid for all sub-devices attached to the ISP anymore. I think it > > > > originally was a separate patch, but they probably have been merged at > > > > some point. I can'tfind it right now anyway. > > > > > > Something like this? > > > > > > Pavel > > > > > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > > > Author: Pavel> > > Date: Wed Mar 1 13:27:56 2017 +0100 > > > > > > Turn bus in struct isp_async_subdev into pointer; some of our subdevs > > > (flash, focus) will not need bus configuration. > > > > > > Signed-off-by: Pavel Machek > > > > I applied this to the ccp2 branch with an improved patch description. > > > > > diff --git a/drivers/media/platform/omap3isp/isp.c > > > b/drivers/media/platform/omap3isp/isp.c index 8a456d4..36bd359 100644 > > > --- a/drivers/media/platform/omap3isp/isp.c > > > +++ b/drivers/media/platform/omap3isp/isp.c > > > @@ -2030,12 +2030,18 @@ enum isp_of_phy { > > > > > > static int isp_fwnode_parse(struct device *dev, struct fwnode_handle > > > *fwn, > > > > > > struct isp_async_subdev *isd) > > > > > > { > > > > > > - struct isp_bus_cfg *buscfg = >bus; > > > + struct isp_bus_cfg *buscfg; > > > > > > struct v4l2_fwnode_endpoint vfwn; > > > unsigned int i; > > > int ret; > > > bool csi1 = false; > > > > > > + buscfg = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL); > > Given that you recently get rid of devm_kzalloc() in the driver, let's not > introduce a new one here. That's certainly a valid point. Still, the entire async sub-devices array is allocated with devm_() allocation functions still; that part wasn't addressed by the patchset mostly removing devm_() memory allocation, so this patch does actually not change how the memory is allocated. Beyond that, I'm not entirely sure whether this is a problem to begin with: devm resources are released after remove() callback and access to this data structure should only happen as a direct result of user IOCTL. IOCTLs may only be in progress as long as there are open file handles on a device --- and such file handles must be closed until the remove() callback may finish. (Referring to the Oslo meeting notes.) Some of the above must be still verified; either way, but the options are clear: either devm must be removed here as well (with the rest) or that it's fine to use it here: from this point of view this patch makes no difference. > > > > + if (!buscfg) > > > + return -ENOMEM; > > > + > > > + isd->bus = buscfg; > > > + > > > ret = v4l2_fwnode_endpoint_parse(fwn, ); > > > if (ret) > > > > > > return ret; > > > > > [snip] > -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi Laurent, On Thu, Mar 02, 2017 at 08:39:51PM +0200, Laurent Pinchart wrote: > Hi Sakari, > > On Thursday 02 Mar 2017 16:16:17 Sakari Ailus wrote: > > On Thu, Mar 02, 2017 at 10:07:27AM +0100, Pavel Machek wrote: > > > Hi! > > > > > > > Making the sub-device bus configuration a pointer should be in a > > > > separate patch. It makes sense since the entire configuration is not > > > > valid for all sub-devices attached to the ISP anymore. I think it > > > > originally was a separate patch, but they probably have been merged at > > > > some point. I can'tfind it right now anyway. > > > > > > Something like this? > > > > > > Pavel > > > > > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > > > Author: Pavel > > > Date: Wed Mar 1 13:27:56 2017 +0100 > > > > > > Turn bus in struct isp_async_subdev into pointer; some of our subdevs > > > (flash, focus) will not need bus configuration. > > > > > > Signed-off-by: Pavel Machek > > > > I applied this to the ccp2 branch with an improved patch description. > > > > > diff --git a/drivers/media/platform/omap3isp/isp.c > > > b/drivers/media/platform/omap3isp/isp.c index 8a456d4..36bd359 100644 > > > --- a/drivers/media/platform/omap3isp/isp.c > > > +++ b/drivers/media/platform/omap3isp/isp.c > > > @@ -2030,12 +2030,18 @@ enum isp_of_phy { > > > > > > static int isp_fwnode_parse(struct device *dev, struct fwnode_handle > > > *fwn, > > > > > > struct isp_async_subdev *isd) > > > > > > { > > > > > > - struct isp_bus_cfg *buscfg = >bus; > > > + struct isp_bus_cfg *buscfg; > > > > > > struct v4l2_fwnode_endpoint vfwn; > > > unsigned int i; > > > int ret; > > > bool csi1 = false; > > > > > > + buscfg = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL); > > Given that you recently get rid of devm_kzalloc() in the driver, let's not > introduce a new one here. That's certainly a valid point. Still, the entire async sub-devices array is allocated with devm_() allocation functions still; that part wasn't addressed by the patchset mostly removing devm_() memory allocation, so this patch does actually not change how the memory is allocated. Beyond that, I'm not entirely sure whether this is a problem to begin with: devm resources are released after remove() callback and access to this data structure should only happen as a direct result of user IOCTL. IOCTLs may only be in progress as long as there are open file handles on a device --- and such file handles must be closed until the remove() callback may finish. (Referring to the Oslo meeting notes.) Some of the above must be still verified; either way, but the options are clear: either devm must be removed here as well (with the rest) or that it's fine to use it here: from this point of view this patch makes no difference. > > > > + if (!buscfg) > > > + return -ENOMEM; > > > + > > > + isd->bus = buscfg; > > > + > > > ret = v4l2_fwnode_endpoint_parse(fwn, ); > > > if (ret) > > > > > > return ret; > > > > > [snip] > -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi Sakari, On Thursday 02 Mar 2017 16:16:17 Sakari Ailus wrote: > On Thu, Mar 02, 2017 at 10:07:27AM +0100, Pavel Machek wrote: > > Hi! > > > > > Making the sub-device bus configuration a pointer should be in a > > > separate patch. It makes sense since the entire configuration is not > > > valid for all sub-devices attached to the ISP anymore. I think it > > > originally was a separate patch, but they probably have been merged at > > > some point. I can'tfind it right now anyway. > > > > Something like this? > > > > Pavel > > > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > > Author: Pavel> > Date: Wed Mar 1 13:27:56 2017 +0100 > > > > Turn bus in struct isp_async_subdev into pointer; some of our subdevs > > (flash, focus) will not need bus configuration. > > > > Signed-off-by: Pavel Machek > > I applied this to the ccp2 branch with an improved patch description. > > > diff --git a/drivers/media/platform/omap3isp/isp.c > > b/drivers/media/platform/omap3isp/isp.c index 8a456d4..36bd359 100644 > > --- a/drivers/media/platform/omap3isp/isp.c > > +++ b/drivers/media/platform/omap3isp/isp.c > > @@ -2030,12 +2030,18 @@ enum isp_of_phy { > > > > static int isp_fwnode_parse(struct device *dev, struct fwnode_handle > > *fwn, > > > > struct isp_async_subdev *isd) > > > > { > > > > - struct isp_bus_cfg *buscfg = >bus; > > + struct isp_bus_cfg *buscfg; > > > > struct v4l2_fwnode_endpoint vfwn; > > unsigned int i; > > int ret; > > bool csi1 = false; > > > > + buscfg = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL); Given that you recently get rid of devm_kzalloc() in the driver, let's not introduce a new one here. > > + if (!buscfg) > > + return -ENOMEM; > > + > > + isd->bus = buscfg; > > + > > ret = v4l2_fwnode_endpoint_parse(fwn, ); > > if (ret) > > > > return ret; > > [snip] -- Regards, Laurent Pinchart
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi Sakari, On Thursday 02 Mar 2017 16:16:17 Sakari Ailus wrote: > On Thu, Mar 02, 2017 at 10:07:27AM +0100, Pavel Machek wrote: > > Hi! > > > > > Making the sub-device bus configuration a pointer should be in a > > > separate patch. It makes sense since the entire configuration is not > > > valid for all sub-devices attached to the ISP anymore. I think it > > > originally was a separate patch, but they probably have been merged at > > > some point. I can'tfind it right now anyway. > > > > Something like this? > > > > Pavel > > > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > > Author: Pavel > > Date: Wed Mar 1 13:27:56 2017 +0100 > > > > Turn bus in struct isp_async_subdev into pointer; some of our subdevs > > (flash, focus) will not need bus configuration. > > > > Signed-off-by: Pavel Machek > > I applied this to the ccp2 branch with an improved patch description. > > > diff --git a/drivers/media/platform/omap3isp/isp.c > > b/drivers/media/platform/omap3isp/isp.c index 8a456d4..36bd359 100644 > > --- a/drivers/media/platform/omap3isp/isp.c > > +++ b/drivers/media/platform/omap3isp/isp.c > > @@ -2030,12 +2030,18 @@ enum isp_of_phy { > > > > static int isp_fwnode_parse(struct device *dev, struct fwnode_handle > > *fwn, > > > > struct isp_async_subdev *isd) > > > > { > > > > - struct isp_bus_cfg *buscfg = >bus; > > + struct isp_bus_cfg *buscfg; > > > > struct v4l2_fwnode_endpoint vfwn; > > unsigned int i; > > int ret; > > bool csi1 = false; > > > > + buscfg = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL); Given that you recently get rid of devm_kzalloc() in the driver, let's not introduce a new one here. > > + if (!buscfg) > > + return -ENOMEM; > > + > > + isd->bus = buscfg; > > + > > ret = v4l2_fwnode_endpoint_parse(fwn, ); > > if (ret) > > > > return ret; > > [snip] -- Regards, Laurent Pinchart
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi Pavel, On Thu, Mar 02, 2017 at 03:58:08PM +0100, Pavel Machek wrote: > Hi! > > > > > Making the sub-device bus configuration a pointer should be in a > > > > separate > > > > patch. It makes sense since the entire configuration is not valid for > > > > all > > > > sub-devices attached to the ISP anymore. I think it originally was a > > > > separate patch, but they probably have been merged at some point. I > > > > can't > > > > find it right now anyway. > > > > > > Something like this? > > > > > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > > > Author: Pavel> > > Date: Wed Mar 1 13:27:56 2017 +0100 > > > > > > Turn bus in struct isp_async_subdev into pointer; some of our subdevs > > > (flash, focus) will not need bus configuration. > > > > > > Signed-off-by: Pavel Machek > > > > I applied this to the ccp2 branch with an improved patch > > description. > > Thanks! > > [But the important part is to get subdevices to work on ccp2 based > branch, and it still fails to work at all if I attempt to enable > them. I'd like to understand why...] Did you add the flash / lens to the async list? The patches currently in the ccp branch do not include that --- it should be in parsing the flash / lens-focus properties in omap3isp device's node. -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi Pavel, On Thu, Mar 02, 2017 at 03:58:08PM +0100, Pavel Machek wrote: > Hi! > > > > > Making the sub-device bus configuration a pointer should be in a > > > > separate > > > > patch. It makes sense since the entire configuration is not valid for > > > > all > > > > sub-devices attached to the ISP anymore. I think it originally was a > > > > separate patch, but they probably have been merged at some point. I > > > > can't > > > > find it right now anyway. > > > > > > Something like this? > > > > > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > > > Author: Pavel > > > Date: Wed Mar 1 13:27:56 2017 +0100 > > > > > > Turn bus in struct isp_async_subdev into pointer; some of our subdevs > > > (flash, focus) will not need bus configuration. > > > > > > Signed-off-by: Pavel Machek > > > > I applied this to the ccp2 branch with an improved patch > > description. > > Thanks! > > [But the important part is to get subdevices to work on ccp2 based > branch, and it still fails to work at all if I attempt to enable > them. I'd like to understand why...] Did you add the flash / lens to the async list? The patches currently in the ccp branch do not include that --- it should be in parsing the flash / lens-focus properties in omap3isp device's node. -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi! > > > Making the sub-device bus configuration a pointer should be in a separate > > > patch. It makes sense since the entire configuration is not valid for all > > > sub-devices attached to the ISP anymore. I think it originally was a > > > separate patch, but they probably have been merged at some point. I can't > > > find it right now anyway. > > > > Something like this? > > > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > > Author: Pavel> > Date: Wed Mar 1 13:27:56 2017 +0100 > > > > Turn bus in struct isp_async_subdev into pointer; some of our subdevs > > (flash, focus) will not need bus configuration. > > > > Signed-off-by: Pavel Machek > > I applied this to the ccp2 branch with an improved patch > description. Thanks! [But the important part is to get subdevices to work on ccp2 based branch, and it still fails to work at all if I attempt to enable them. I'd like to understand why...] Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi! > > > Making the sub-device bus configuration a pointer should be in a separate > > > patch. It makes sense since the entire configuration is not valid for all > > > sub-devices attached to the ISP anymore. I think it originally was a > > > separate patch, but they probably have been merged at some point. I can't > > > find it right now anyway. > > > > Something like this? > > > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > > Author: Pavel > > Date: Wed Mar 1 13:27:56 2017 +0100 > > > > Turn bus in struct isp_async_subdev into pointer; some of our subdevs > > (flash, focus) will not need bus configuration. > > > > Signed-off-by: Pavel Machek > > I applied this to the ccp2 branch with an improved patch > description. Thanks! [But the important part is to get subdevices to work on ccp2 based branch, and it still fails to work at all if I attempt to enable them. I'd like to understand why...] Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi Pavel, On Thu, Mar 02, 2017 at 10:07:27AM +0100, Pavel Machek wrote: > Hi! > > > Making the sub-device bus configuration a pointer should be in a separate > > patch. It makes sense since the entire configuration is not valid for all > > sub-devices attached to the ISP anymore. I think it originally was a > > separate patch, but they probably have been merged at some point. I can't > > find it right now anyway. > > Something like this? > Pavel > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > Author: Pavel> Date: Wed Mar 1 13:27:56 2017 +0100 > > Turn bus in struct isp_async_subdev into pointer; some of our subdevs > (flash, focus) will not need bus configuration. > > Signed-off-by: Pavel Machek I applied this to the ccp2 branch with an improved patch description. > > diff --git a/drivers/media/platform/omap3isp/isp.c > b/drivers/media/platform/omap3isp/isp.c > index 8a456d4..36bd359 100644 > --- a/drivers/media/platform/omap3isp/isp.c > +++ b/drivers/media/platform/omap3isp/isp.c > @@ -2030,12 +2030,18 @@ enum isp_of_phy { > static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwn, > struct isp_async_subdev *isd) > { > - struct isp_bus_cfg *buscfg = >bus; > + struct isp_bus_cfg *buscfg; > struct v4l2_fwnode_endpoint vfwn; > unsigned int i; > int ret; > bool csi1 = false; > > + buscfg = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL); > + if (!buscfg) > + return -ENOMEM; > + > + isd->bus = buscfg; > + > ret = v4l2_fwnode_endpoint_parse(fwn, ); > if (ret) > return ret; > @@ -2246,7 +2252,7 @@ static int isp_subdev_notifier_bound(struct > v4l2_async_notifier *async, > container_of(asd, struct isp_async_subdev, asd); > > isd->sd = subdev; > - isd->sd->host_priv = >bus; > + isd->sd->host_priv = isd->bus; > > return 0; > } > diff --git a/drivers/media/platform/omap3isp/isp.h > b/drivers/media/platform/omap3isp/isp.h > index 7e6f663..c0b9d1d 100644 > --- a/drivers/media/platform/omap3isp/isp.h > +++ b/drivers/media/platform/omap3isp/isp.h > @@ -228,7 +228,7 @@ struct isp_device { > > struct isp_async_subdev { > struct v4l2_subdev *sd; > - struct isp_bus_cfg bus; > + struct isp_bus_cfg *bus; > struct v4l2_async_subdev asd; > }; > > diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c > b/drivers/media/platform/omap3isp/ispcsiphy.c > index f20abe8..be23408 100644 > --- a/drivers/media/platform/omap3isp/ispcsiphy.c > +++ b/drivers/media/platform/omap3isp/ispcsiphy.c > @@ -202,7 +202,7 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy) > struct isp_async_subdev *isd = > container_of(pipe->external->asd, >struct isp_async_subdev, asd); > - buscfg = >bus; > + buscfg = isd->bus; > } > > if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1 > > -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: subdevice config into pointer (was Re: [PATCH 1/4] v4l2: device_register_subdev_nodes: allow calling multiple times)
Hi Pavel, On Thu, Mar 02, 2017 at 10:07:27AM +0100, Pavel Machek wrote: > Hi! > > > Making the sub-device bus configuration a pointer should be in a separate > > patch. It makes sense since the entire configuration is not valid for all > > sub-devices attached to the ISP anymore. I think it originally was a > > separate patch, but they probably have been merged at some point. I can't > > find it right now anyway. > > Something like this? > Pavel > > commit df9141c66678b549fac9d143bd55ed0b242cf36e > Author: Pavel > Date: Wed Mar 1 13:27:56 2017 +0100 > > Turn bus in struct isp_async_subdev into pointer; some of our subdevs > (flash, focus) will not need bus configuration. > > Signed-off-by: Pavel Machek I applied this to the ccp2 branch with an improved patch description. > > diff --git a/drivers/media/platform/omap3isp/isp.c > b/drivers/media/platform/omap3isp/isp.c > index 8a456d4..36bd359 100644 > --- a/drivers/media/platform/omap3isp/isp.c > +++ b/drivers/media/platform/omap3isp/isp.c > @@ -2030,12 +2030,18 @@ enum isp_of_phy { > static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwn, > struct isp_async_subdev *isd) > { > - struct isp_bus_cfg *buscfg = >bus; > + struct isp_bus_cfg *buscfg; > struct v4l2_fwnode_endpoint vfwn; > unsigned int i; > int ret; > bool csi1 = false; > > + buscfg = devm_kzalloc(dev, sizeof(*isd->bus), GFP_KERNEL); > + if (!buscfg) > + return -ENOMEM; > + > + isd->bus = buscfg; > + > ret = v4l2_fwnode_endpoint_parse(fwn, ); > if (ret) > return ret; > @@ -2246,7 +2252,7 @@ static int isp_subdev_notifier_bound(struct > v4l2_async_notifier *async, > container_of(asd, struct isp_async_subdev, asd); > > isd->sd = subdev; > - isd->sd->host_priv = >bus; > + isd->sd->host_priv = isd->bus; > > return 0; > } > diff --git a/drivers/media/platform/omap3isp/isp.h > b/drivers/media/platform/omap3isp/isp.h > index 7e6f663..c0b9d1d 100644 > --- a/drivers/media/platform/omap3isp/isp.h > +++ b/drivers/media/platform/omap3isp/isp.h > @@ -228,7 +228,7 @@ struct isp_device { > > struct isp_async_subdev { > struct v4l2_subdev *sd; > - struct isp_bus_cfg bus; > + struct isp_bus_cfg *bus; > struct v4l2_async_subdev asd; > }; > > diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c > b/drivers/media/platform/omap3isp/ispcsiphy.c > index f20abe8..be23408 100644 > --- a/drivers/media/platform/omap3isp/ispcsiphy.c > +++ b/drivers/media/platform/omap3isp/ispcsiphy.c > @@ -202,7 +202,7 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy) > struct isp_async_subdev *isd = > container_of(pipe->external->asd, >struct isp_async_subdev, asd); > - buscfg = >bus; > + buscfg = isd->bus; > } > > if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1 > > -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk