linux-kernel - Re: [PATCH 2/2 v16] drm/bridge: Add I2C based driver for ps8640 bridge

Open Source and information security mailing list archives

Message-ID: <20160804105814.GH6232@phenom.ffwll.local>
Date:	Thu, 4 Aug 2016 12:58:15 +0200
From:	Daniel Vetter <daniel@...ll.ch>
To:	Enric Balletbo Serra <eballetbo@...il.com>
Cc:	Daniel Vetter <daniel@...ll.ch>,
	Daniel Kurtz <djkurtz@...omium.org>,
	Mark Rutland <mark.rutland@....com>, stonea168@....com,
	ML dri-devel <dri-devel@...ts.freedesktop.org>,
	Ajay Kumar <ajaykumar.rs@...sung.com>,
	Vincent Palatin <vpalatin@...omium.org>,
	cawa cheng <cawa.cheng@...iatek.com>,
	Russell King <rmk+kernel@....linux.org.uk>,
	Thierry Reding <treding@...dia.com>,
	devicetree <devicetree@...r.kernel.org>,
	Jitao Shi <jitao.shi@...iatek.com>,
	Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Rob Herring <robh+dt@...nel.org>,
	"moderated list:ARM/Mediatek SoC support" 
	<linux-mediatek@...ts.infradead.org>,
	Yingjoe Chen <yingjoe.chen@...iatek.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	Eddie Huang (黃智傑) 
	<eddie.huang@...iatek.com>,
	LAKML <linux-arm-kernel@...ts.infradead.org>,
	Rahul Sharma <rahul.sharma@...sung.com>,
	srv_heupstream <srv_heupstream@...iatek.com>,
	Emil Velikov <emil.l.velikov@...il.com>,
	"Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>,
	Sascha Hauer <kernel@...gutronix.de>,
	Kumar Gala <galak@...eaurora.org>,
	Andy Yan <andy.yan@...k-chips.com>
Subject: Re: [PATCH 2/2 v16] drm/bridge: Add I2C based driver for ps8640
 bridge

On Thu, Aug 04, 2016 at 12:35:59PM +0200, Enric Balletbo Serra wrote:
> 2016-07-12 12:13 GMT+02:00 Daniel Vetter <daniel@...ll.ch>:
> > On Wed, Jun 29, 2016 at 6:31 AM, Daniel Kurtz <djkurtz@...omium.org> wrote:
> >> On Fri, Jun 17, 2016 at 3:14 AM, Emil Velikov <emil.l.velikov@...il.com> wrote:
> >>>> +static ssize_t ps8640_update_fw_store(struct device *dev,
> >>>> +                                     struct device_attribute *attr,
> >>>> +                                     const char *buf, size_t count)
> >>>> +{
> >>>> +       struct i2c_client *client = to_i2c_client(dev);
> >>>> +       struct ps8640 *ps_bridge = i2c_get_clientdata(client);
> >>>> +       const struct firmware *fw;
> >>>> +       int error;
> >>>> +
> >>>> +       error = request_firmware(&fw, PS_FW_NAME, dev);
> >>> Can the device operate without a firmware ? If not, why is the
> >>> firmware loaded so later/after user interaction (via sysfs) ? I don't
> >>> recall any other driver in DRM to use such an approach.
> >>
> >> The PS8640 has internal flash, so it should always already have a
> >> working firmware.
> >> This sysfs interface is useful for user space initiated field firmware updates.
> >
> > Might be better to just do a request_firmware on driver load, and
> > simply proceed if it's not there. Adding a sysfs interface (which is
> > abi) seems way too much overkill for this imo. If you want to upgrade
> > the firmware you can then just drop it into the right directory, with
> > no further interaction needed.
> 
> IMHO I'm not sure if for this use case request_firmware on driver load
> is a good idea. Flash the non-volatile internal chip can be a slow
> operation and if you forget to remove the firmware after drop it into
> the right directory apart from slow down the driver probe you can
> damage the chip depending on the write endurance of the chip.

Ah ok. Explaining this in the commit message would be really good.

> This sysfs interface is used on other subsystems when there is a
> non-volatile memory, as example you can see at [1], [2]. Unfortunately
> not all are using the same sysfs interface and maybe this should be
> standardized (maybe it's an opportunity to define it)

+1 on standardizing this.
-Daniel
> 
> Regards,
>  Enric
> 
> [1] http://lxr.free-electrons.com/source/drivers/input/touchscreen/wdt87xx_i2c.c#L922
> [2] http://lxr.free-electrons.com/source/drivers/scsi/pm8001/pm8001_ctl.c#L732
> 
> 
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@...ts.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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