On Mon, 10 Mar 2014, Thierry Reding thierry.reding@gmail.com wrote:
On Wed, Mar 05, 2014 at 01:50:27PM +0200, Jani Nikula wrote: [...]
I am not entirely happy with adding an extra name field, but I also didn't like the caller having to set up aux.ddc.name in advance. Ideas welcome.
What you propose is a whole lot better than having to modify the internals of struct i2c_adapter in advance, and I can't think of a better alternative either. I have two small comments below.
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 35251af3b14e..17832d048147 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -726,7 +726,8 @@ int drm_dp_aux_register_i2c_bus(struct drm_dp_aux *aux) aux->ddc.dev.parent = aux->dev; aux->ddc.dev.of_node = aux->dev->of_node;
- strncpy(aux->ddc.name, dev_name(aux->dev), sizeof(aux->ddc.name));
- strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
sizeof(aux->ddc.name));I don't see why the change from strncpy() to strlcpy() would be necessary.
To always null terminate aux->ddc.name.
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
[...]
@@ -438,6 +438,9 @@ struct drm_dp_aux_msg {
- The .dev field should be set to a pointer to the device that implements
- the AUX channel.
- The .name field may be used to specify the name of the I2C adapter. If set to
- NULL, dev_name() of .dev will be used.
Nit: That new paragraph sticks out because it's wider than the rest, even though it still fits into 80 characters.
It seems emacs isn't (yet!) clever enough to reflow with the same fill-column as the neighbouring paragraphs. ;)
I don't feel all that strongly about either of the above, so:
Reviewed-by: Thierry Reding treding@nvidia.com
Thanks for the review, Jani.