Re: [PATCH v2 6/6] drm/panel: rpi-touchscreen: Set status to "fail" when ->probe() fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, May 3, 2018 at 11:40 AM, Boris Brezillon
<boris.brezillon@xxxxxxxxxxx> wrote:
> The device might be described in the device tree but not connected to
> the I2C bus. Update the status property so that the DRM panel logic
> returns -ENODEV when someone tries to get the panel attached to this
> DT node.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>
> ---
>  .../gpu/drm/panel/panel-raspberrypi-touchscreen.c  | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> index 2c9c9722734f..b8fcb1acef75 100644
> --- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> +++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
> @@ -358,6 +358,39 @@ static const struct drm_panel_funcs rpi_touchscreen_funcs = {
>         .get_modes = rpi_touchscreen_get_modes,
>  };
>
> +static void rpi_touchscreen_set_status_fail(struct i2c_client *i2c)
> +{
> +       struct property *newprop;
> +
> +       newprop = kzalloc(sizeof(*newprop), GFP_KERNEL);
> +       if (!newprop)
> +               return;
> +
> +       newprop->name = kstrdup("status", GFP_KERNEL);
> +       if (!newprop->name)
> +               goto err;
> +
> +       newprop->value = kstrdup("fail", GFP_KERNEL);
> +       if (!newprop->value)
> +               goto err;
> +
> +       newprop->length = sizeof("fail");
> +
> +       if (of_update_property(i2c->dev.of_node, newprop))
> +               goto err;
> +

As I mentioned on irc, can you make this a common DT function.

I'm not sure if it matters that we set status to fail vs. disabled. I
somewhat prefer the latter as we already have other cases and I'd
rather the api not pass a string in. I can't think of any reason to
distinguish the difference between fail and disabled.

> +       /* We intentionally leak the memory we allocate here, because the new
> +        * OF property might live longer than the underlying dev, so no way
> +        * we can use devm_kzalloc() here.
> +        */
> +       return;
> +
> +err:
> +       kfree(newprop->value);
> +       kfree(newprop->name);
> +       kfree(newprop);
> +}
> +
>  static int rpi_touchscreen_probe(struct i2c_client *i2c,
>                                  const struct i2c_device_id *id)
>  {
> @@ -382,6 +415,7 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c,
>
>         ver = rpi_touchscreen_i2c_read(ts, REG_ID);
>         if (ver < 0) {
> +               rpi_touchscreen_set_status_fail(i2c);

I've thought some more about this and I still think this should be
handled in the driver core or i2c core.

The reason is simple. I think the state of the system should be the
same after this as if you booted with 'status = "disabled"' for this
node. And that means the device should be removed completely because
we don't create struct device's for disabled nodes.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux