From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753885AbcGLKNz (ORCPT ); Tue, 12 Jul 2016 06:13:55 -0400 Received: from mail-oi0-f65.google.com ([209.85.218.65]:36325 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753568AbcGLKNx (ORCPT ); Tue, 12 Jul 2016 06:13:53 -0400 MIME-Version: 1.0 X-Originating-IP: [2a02:168:56b5:0:ac27:b86c:7764:9429] In-Reply-To: References: <1464861445-11086-1-git-send-email-jitao.shi@mediatek.com> <1464861445-11086-2-git-send-email-jitao.shi@mediatek.com> From: Daniel Vetter Date: Tue, 12 Jul 2016 12:13:52 +0200 X-Google-Sender-Auth: 0Z07hf85kDKP1qF6HqBVdv7b2vA Message-ID: Subject: Re: [PATCH 2/2 v16] drm/bridge: Add I2C based driver for ps8640 bridge To: Daniel Kurtz Cc: Emil Velikov , Mark Rutland , stonea168@163.com, ML dri-devel , Ajay Kumar , Vincent Palatin , cawa cheng , Yingjoe Chen , Thierry Reding , devicetree , Jitao Shi , Pawel Moll , Ian Campbell , Rob Herring , "moderated list:ARM/Mediatek SoC support" , Russell King , Matthias Brugger , =?UTF-8?B?RWRkaWUgSHVhbmcgKOm7g+aZuuWCkSk=?= , LAKML , Rahul Sharma , srv_heupstream , "Linux-Kernel@Vger. Kernel. Org" , Sascha Hauer , Kumar Gala , Andy Yan Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 29, 2016 at 6:31 AM, Daniel Kurtz wrote: > On Fri, Jun 17, 2016 at 3:14 AM, Emil Velikov 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. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/2 v16] drm/bridge: Add I2C based driver for ps8640 bridge Date: Tue, 12 Jul 2016 12:13:52 +0200 Message-ID: References: <1464861445-11086-1-git-send-email-jitao.shi@mediatek.com> <1464861445-11086-2-git-send-email-jitao.shi@mediatek.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Kurtz Cc: Mark Rutland , stonea168@163.com, ML dri-devel , Ajay Kumar , Vincent Palatin , cawa cheng , Russell King , Thierry Reding , devicetree , Jitao Shi , Pawel Moll , Ian Campbell , Rob Herring , "moderated list:ARM/Mediatek SoC support" , Yingjoe Chen , Matthias Brugger , =?UTF-8?B?RWRkaWUgSHVhbmcgKOm7g+aZuuWCkSk=?= , LAKML , Rahul Sharma , srv_heupstream , Emil Velikov List-Id: devicetree@vger.kernel.org T24gV2VkLCBKdW4gMjksIDIwMTYgYXQgNjozMSBBTSwgRGFuaWVsIEt1cnR6IDxkamt1cnR6QGNo cm9taXVtLm9yZz4gd3JvdGU6Cj4gT24gRnJpLCBKdW4gMTcsIDIwMTYgYXQgMzoxNCBBTSwgRW1p bCBWZWxpa292IDxlbWlsLmwudmVsaWtvdkBnbWFpbC5jb20+IHdyb3RlOgo+Pj4gK3N0YXRpYyBz c2l6ZV90IHBzODY0MF91cGRhdGVfZndfc3RvcmUoc3RydWN0IGRldmljZSAqZGV2LAo+Pj4gKyAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzdHJ1Y3QgZGV2aWNlX2F0dHJpYnV0 ZSAqYXR0ciwKPj4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgY29uc3Qg Y2hhciAqYnVmLCBzaXplX3QgY291bnQpCj4+PiArewo+Pj4gKyAgICAgICBzdHJ1Y3QgaTJjX2Ns aWVudCAqY2xpZW50ID0gdG9faTJjX2NsaWVudChkZXYpOwo+Pj4gKyAgICAgICBzdHJ1Y3QgcHM4 NjQwICpwc19icmlkZ2UgPSBpMmNfZ2V0X2NsaWVudGRhdGEoY2xpZW50KTsKPj4+ICsgICAgICAg Y29uc3Qgc3RydWN0IGZpcm13YXJlICpmdzsKPj4+ICsgICAgICAgaW50IGVycm9yOwo+Pj4gKwo+ Pj4gKyAgICAgICBlcnJvciA9IHJlcXVlc3RfZmlybXdhcmUoJmZ3LCBQU19GV19OQU1FLCBkZXYp Owo+PiBDYW4gdGhlIGRldmljZSBvcGVyYXRlIHdpdGhvdXQgYSBmaXJtd2FyZSA/IElmIG5vdCwg d2h5IGlzIHRoZQo+PiBmaXJtd2FyZSBsb2FkZWQgc28gbGF0ZXIvYWZ0ZXIgdXNlciBpbnRlcmFj dGlvbiAodmlhIHN5c2ZzKSA/IEkgZG9uJ3QKPj4gcmVjYWxsIGFueSBvdGhlciBkcml2ZXIgaW4g RFJNIHRvIHVzZSBzdWNoIGFuIGFwcHJvYWNoLgo+Cj4gVGhlIFBTODY0MCBoYXMgaW50ZXJuYWwg Zmxhc2gsIHNvIGl0IHNob3VsZCBhbHdheXMgYWxyZWFkeSBoYXZlIGEKPiB3b3JraW5nIGZpcm13 YXJlLgo+IFRoaXMgc3lzZnMgaW50ZXJmYWNlIGlzIHVzZWZ1bCBmb3IgdXNlciBzcGFjZSBpbml0 aWF0ZWQgZmllbGQgZmlybXdhcmUgdXBkYXRlcy4KCk1pZ2h0IGJlIGJldHRlciB0byBqdXN0IGRv IGEgcmVxdWVzdF9maXJtd2FyZSBvbiBkcml2ZXIgbG9hZCwgYW5kCnNpbXBseSBwcm9jZWVkIGlm IGl0J3Mgbm90IHRoZXJlLiBBZGRpbmcgYSBzeXNmcyBpbnRlcmZhY2UgKHdoaWNoIGlzCmFiaSkg c2VlbXMgd2F5IHRvbyBtdWNoIG92ZXJraWxsIGZvciB0aGlzIGltby4gSWYgeW91IHdhbnQgdG8g dXBncmFkZQp0aGUgZmlybXdhcmUgeW91IGNhbiB0aGVuIGp1c3QgZHJvcCBpdCBpbnRvIHRoZSBy aWdodCBkaXJlY3RvcnksIHdpdGgKbm8gZnVydGhlciBpbnRlcmFjdGlvbiBuZWVkZWQuCi1EYW5p ZWwKLS0gCkRhbmllbCBWZXR0ZXIKU29mdHdhcmUgRW5naW5lZXIsIEludGVsIENvcnBvcmF0aW9u Cis0MSAoMCkgNzkgMzY1IDU3IDQ4IC0gaHR0cDovL2Jsb2cuZmZ3bGwuY2gKX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlz dApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0 b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel@ffwll.ch (Daniel Vetter) Date: Tue, 12 Jul 2016 12:13:52 +0200 Subject: [PATCH 2/2 v16] drm/bridge: Add I2C based driver for ps8640 bridge In-Reply-To: References: <1464861445-11086-1-git-send-email-jitao.shi@mediatek.com> <1464861445-11086-2-git-send-email-jitao.shi@mediatek.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, Jun 29, 2016 at 6:31 AM, Daniel Kurtz wrote: > On Fri, Jun 17, 2016 at 3:14 AM, Emil Velikov 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. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch