Em Thu, 15 Oct 2015 00:39:32 +0300 Sakari Ailus sakari.ailus@iki.fi escreveu:
Mauro Carvalho Chehab wrote:
Em Wed, 14 Oct 2015 13:07:21 +0200 Javier Martinez Canillas javier@osg.samsung.com escreveu:
Hello Sakari,
On 10/14/2015 01:01 PM, Sakari Ailus wrote:
On Wed, Oct 14, 2015 at 12:56:54PM +0200, Javier Martinez Canillas wrote:
Hello,
On 10/14/2015 12:09 PM, Sakari Ailus wrote:
On Mon, Oct 12, 2015 at 08:52:43PM -0300, Mauro Carvalho Chehab wrote: > Sakari Ailus sakari.ailus@iki.fi escreveu:
[snip]
>>> >>> static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async) >>> { >>> struct isp_device *isp = container_of(async, struct isp_device, >>> notifier); >>> + struct v4l2_device *v4l2_dev = &isp->v4l2_dev; >>> + struct v4l2_subdev *sd; >>> + struct isp_bus_cfg *bus; >>> + int ret; >>> + >>> + list_for_each_entry(sd, &v4l2_dev->subdevs, list) { >> >> list_for_each_entry(sd, &isp->v4l2_dev.subdevs, list) { >> >> And you can drop local v4l2_dev. > > This is Javier's patch, but I agree that this can be simplified. >
Yes, the local variable is not needed but usually I prefer to use a variable when two levels of indirection are needed since I think that makes the code easier to read.
I don't have a strong opinion though so I don't mind if is dropped.
Later on we have:
return v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
At least consistency should be maintained, whichever choice we make here. :-)
That's true, sorry about that.
Feel free to resubmit, but I can do that as well.
I prefer to wait for Mauro's opinion since maybe he would prefer to have that changed on top of the series to avoid having him to do a rebase due a new version of $SUBJECT being resubmit.
I prefer a latter patch doing such cleanups.
Well... I don't see rebasing as risky as you do so I'd simply fix the patch that would need to be fixed, but I'm ok with fixing that later once this series is eventually in.
It is, specially with renames, as all patches touching the code nearby will be broken, and will require manual rework and compilation retest.
But when?
After the 83 patches, before merging at linux_tree "master" branch.
Could you reply my other comments on patch 49, "uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl"?
Just replied.
Also, naming is one of the things that are difficult to get right, yet it is still important (patch 28, "media: add support to link interfaces and entities").
Yes, no doubt that naming is important, but it is something that can be changed anytime as needed, as a rename patch won't break anything, nor change the functionality, except for patches that might be relying on it. As Shuah made some some ALSA patches on the top of this 83 patch series, we should first merge the existing stuff and then apply whatever renaming patches we think it is needed, in order to avoid uneeded rework.
Regards, Mauro