lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 19 Dec 2020 20:52:29 +0200 From: Andy Shevchenko <andy.shevchenko@...il.com> To: Daniel Scally <djrscally@...il.com> Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, ACPI Devel Maling List <linux-acpi@...r.kernel.org>, Linux Media Mailing List <linux-media@...r.kernel.org>, devel@...ica.org, "Rafael J. Wysocki" <rjw@...ysocki.net>, Len Brown <lenb@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Yong Zhi <yong.zhi@...el.com>, Sakari Ailus <sakari.ailus@...ux.intel.com>, Bingbu Cao <bingbu.cao@...el.com>, Tian Shu Qiu <tian.shu.qiu@...el.com>, Mauro Carvalho Chehab <mchehab@...nel.org>, Robert Moore <robert.moore@...el.com>, Erik Kaneda <erik.kaneda@...el.com>, Petr Mladek <pmladek@...e.com>, Steven Rostedt <rostedt@...dmis.org>, Sergey Senozhatsky <sergey.senozhatsky@...il.com>, Rasmus Villemoes <linux@...musvillemoes.dk>, Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>, Jacopo Mondi <jacopo+renesas@...ndi.org>, kieran.bingham+renesas@...asonboard.com, Linus Walleij <linus.walleij@...aro.org>, "Krogerus, Heikki" <heikki.krogerus@...ux.intel.com>, Tsuchiya Yuto <kitakar@...il.com>, jorhand@...ux.microsoft.com Subject: Re: [PATCH v2 12/12] ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver On Sat, Dec 19, 2020 at 2:25 AM Daniel Scally <djrscally@...il.com> wrote: > On 18/12/2020 21:17, Andy Shevchenko wrote: > > On Thu, Dec 17, 2020 at 11:43:37PM +0000, Daniel Scally wrote: ... > >> + sensor->ep_properties[0] = PROPERTY_ENTRY_U32(sensor->prop_names.bus_type, 4); > > > > Does 4 has any meaning that can be described by #define ? > > It's V4L2_FWNODE_BUS_TYPE_CSI2_DPHY: > > https://elixir.bootlin.com/linux/latest/source/drivers/media/v4l2-core/v4l2-fwnode.c#L36 > > That enum's not in an accessible header, but I can define it in this > module's header Maybe you can do a preparatory patch to make it visible to v4l2 drivers? (Like moving to one of v4l2 headers) ... > >> + if (bridge->n_sensors >= CIO2_NUM_PORTS) { > >> + dev_warn(&cio2->dev, "Exceeded available CIO2 ports\n"); > > > >> + /* overflow i so outer loop ceases */ > >> + i = ARRAY_SIZE(cio2_supported_sensors); > >> + break; > > > > Why not to create a new label below and assign ret here with probably comment > > why it's not an error? > > Sure, I can do that, but since it wouldn't need any cleanup I could also > just return 0 here as Laurent suggest (but with a comment explaining why > that's ok as you say) - do you have a preference? While it's a good suggestion it will bring a bit of inconsistency into approach. Everywhere else in the function you are using the goto approach. So yes, I have a preference. > >> + } ... > >> + ret = cio2_bridge_read_acpi_buffer(adev, "SSDB", > >> + &sensor->ssdb, > >> + sizeof(sensor->ssdb)); > >> + if (ret < 0) > > > > if (ret) (because positive case can be returned just by next conditional). > > cio2_bridge_read_acpi_buffer() returns the buffer length on success at > the moment, but I can change it to return 0 and have this be if (ret) Please correct this somehow, because the next failure returns it instead of error... > >> + goto err_put_adev; > >> + > >> + if (sensor->ssdb.lanes > 4) { > >> + dev_err(&adev->dev, > >> + "Number of lanes in SSDB is invalid\n"); ...I'm even thinking that you have to assign ret here to something meaningful. > >> + goto err_put_adev; > >> + } -- With Best Regards, Andy Shevchenko
Powered by blists - more mailing lists