From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752177AbcBCKhh (ORCPT ); Wed, 3 Feb 2016 05:37:37 -0500 Received: from mail-vk0-f49.google.com ([209.85.213.49]:34855 "EHLO mail-vk0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750750AbcBCKhb (ORCPT ); Wed, 3 Feb 2016 05:37:31 -0500 MIME-Version: 1.0 In-Reply-To: <1454489288-31951-2-git-send-email-jitao.shi@mediatek.com> References: <1454489288-31951-1-git-send-email-jitao.shi@mediatek.com> <1454489288-31951-2-git-send-email-jitao.shi@mediatek.com> From: Daniel Kurtz Date: Wed, 3 Feb 2016 18:37:10 +0800 X-Google-Sender-Auth: 77yhcFjhc50yy5g3uXmY18eU9H0 Message-ID: Subject: Re: [PATCH v9 2/2] drm/bridge: Add I2C based driver for ps8640 bridge To: Jitao Shi Cc: David Airlie , Mark Rutland , stonea168@163.com, dri-devel , Ajay Kumar , Vincent Palatin , cawa cheng , =?UTF-8?B?QmliYnkgSHNpZWggKOisnea/n+mBoCk=?= , CK HU , Russell King , Thierry Reding , Sean Paul , "open list:OPEN FIRMWARE AND..." , Sascha Hauer , Pawel Moll , Ian Campbell , Inki Dae , Rob Herring , "moderated list:ARM/Mediatek SoC support" , Yingjoe Chen , Matthias Brugger , =?UTF-8?B?RWRkaWUgSHVhbmcgKOm7g+aZuuWCkSk=?= , "linux-arm-kernel@lists.infradead.org" , Rahul Sharma , srv_heupstream , "linux-kernel@vger.kernel.org" , Philipp Zabel , 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 Hi Jitao, Looks really good. Just a couple of tiny things... On Wed, Feb 3, 2016 at 4:48 PM, Jitao Shi wrote: > > This patch adds drm_bridge driver for parade DSI to eDP bridge chip. > > Signed-off-by: Jitao Shi > --- [snip] > +static int ps8640_get_modes(struct drm_connector *connector) > +{ > + struct ps8640 *ps_bridge = connector_to_ps8640(connector); > + u8 *edid; > + int ret, num_modes = 0; > + bool power_off; > + > + if (ps_bridge->edid) > + return drm_add_edid_modes(connector, ps_bridge->edid); > + > + power_off = !ps_bridge->enabled; > + ps8640_pre_enable(&ps_bridge->bridge); > + > + edid = kmalloc(EDID_LENGTH, GFP_KERNEL); devm_kmalloc() if you can. Or at least kfree() in ps8640_remove > + if (!edid) { > + DRM_ERROR("Failed to allocate EDID\n"); > + return 0; > + } > + > + ret = ps8640_read(ps_bridge->ddc_i2c, 0, edid, EDID_LENGTH); > + if (ret) { > + kfree(edid); > + goto out; > + } > + > + ps_bridge->edid = (struct edid *)edid; > + drm_mode_connector_update_edid_property(connector, ps_bridge->edid); > + > + num_modes = drm_add_edid_modes(connector, ps_bridge->edid); > + > +out: > + if (power_off) > + ps8640_post_disable(&ps_bridge->bridge); > + > + return num_modes; > +} [snip] > +static int ps8640_spi_normal_mode(struct ps8640 *ps_bridge) > +{ > + u8 cmd[2]; > + struct i2c_client *client = ps_bridge->page[2]; > + > + /* Enable-Write-Status-Register */ > + cmd[0] = WRITE_ENABLE_CMD; > + ps8640_spi_send_cmd(ps_bridge, cmd, 1); > + > + /* protect BPL/BP0/BP1 */ > + cmd[0] = WRITE_STATUS_REG_CMD; > + cmd[1] = BLK_PROTECT_BITS | STATUS_REG_PROTECT; > + ps8640_spi_send_cmd(ps_bridge, cmd, 2); > + > + /* wait for SPI rom ready */ > + ps8640_wait_rom_idle(ps_bridge); > + > + /* disable PS8640 mapping function */ > + ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, 0x00); > + > + gpiod_set_value(ps_bridge->gpio_mode_sel_n, 1); > + return 0; > +} > + > +static int ps8640_enter_bl(struct ps8640 *ps_bridge) > +{ > + ps_bridge->in_fw_update = true; > + return ps8640_spi_dl_mode(ps_bridge); On error: ps_bridge->in_fw_update = false; Thanks, -Dan From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Kurtz Subject: Re: [PATCH v9 2/2] drm/bridge: Add I2C based driver for ps8640 bridge Date: Wed, 3 Feb 2016 18:37:10 +0800 Message-ID: References: <1454489288-31951-1-git-send-email-jitao.shi@mediatek.com> <1454489288-31951-2-git-send-email-jitao.shi@mediatek.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <1454489288-31951-2-git-send-email-jitao.shi-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Jitao Shi Cc: David Airlie , Mark Rutland , stonea168-9Onoh4P/yGk@public.gmane.org, dri-devel , Ajay Kumar , Vincent Palatin , cawa cheng , =?UTF-8?B?QmliYnkgSHNpZWggKOisnea/n+mBoCk=?= , CK HU , Russell King , Thierry Reding , Sean Paul , "open list:OPEN FIRMWARE AND..." , Sascha Hauer , Pawel Moll , Ian Campbell , Inki Dae , Rob Herring , "moderated list:ARM/Mediatek SoC support" , Yingjoe Chen , Matthias Brugger List-Id: devicetree@vger.kernel.org Hi Jitao, Looks really good. Just a couple of tiny things... On Wed, Feb 3, 2016 at 4:48 PM, Jitao Shi wrote: > > This patch adds drm_bridge driver for parade DSI to eDP bridge chip. > > Signed-off-by: Jitao Shi > --- [snip] > +static int ps8640_get_modes(struct drm_connector *connector) > +{ > + struct ps8640 *ps_bridge = connector_to_ps8640(connector); > + u8 *edid; > + int ret, num_modes = 0; > + bool power_off; > + > + if (ps_bridge->edid) > + return drm_add_edid_modes(connector, ps_bridge->edid); > + > + power_off = !ps_bridge->enabled; > + ps8640_pre_enable(&ps_bridge->bridge); > + > + edid = kmalloc(EDID_LENGTH, GFP_KERNEL); devm_kmalloc() if you can. Or at least kfree() in ps8640_remove > + if (!edid) { > + DRM_ERROR("Failed to allocate EDID\n"); > + return 0; > + } > + > + ret = ps8640_read(ps_bridge->ddc_i2c, 0, edid, EDID_LENGTH); > + if (ret) { > + kfree(edid); > + goto out; > + } > + > + ps_bridge->edid = (struct edid *)edid; > + drm_mode_connector_update_edid_property(connector, ps_bridge->edid); > + > + num_modes = drm_add_edid_modes(connector, ps_bridge->edid); > + > +out: > + if (power_off) > + ps8640_post_disable(&ps_bridge->bridge); > + > + return num_modes; > +} [snip] > +static int ps8640_spi_normal_mode(struct ps8640 *ps_bridge) > +{ > + u8 cmd[2]; > + struct i2c_client *client = ps_bridge->page[2]; > + > + /* Enable-Write-Status-Register */ > + cmd[0] = WRITE_ENABLE_CMD; > + ps8640_spi_send_cmd(ps_bridge, cmd, 1); > + > + /* protect BPL/BP0/BP1 */ > + cmd[0] = WRITE_STATUS_REG_CMD; > + cmd[1] = BLK_PROTECT_BITS | STATUS_REG_PROTECT; > + ps8640_spi_send_cmd(ps_bridge, cmd, 2); > + > + /* wait for SPI rom ready */ > + ps8640_wait_rom_idle(ps_bridge); > + > + /* disable PS8640 mapping function */ > + ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, 0x00); > + > + gpiod_set_value(ps_bridge->gpio_mode_sel_n, 1); > + return 0; > +} > + > +static int ps8640_enter_bl(struct ps8640 *ps_bridge) > +{ > + ps_bridge->in_fw_update = true; > + return ps8640_spi_dl_mode(ps_bridge); On error: ps_bridge->in_fw_update = false; Thanks, -Dan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: djkurtz@chromium.org (Daniel Kurtz) Date: Wed, 3 Feb 2016 18:37:10 +0800 Subject: [PATCH v9 2/2] drm/bridge: Add I2C based driver for ps8640 bridge In-Reply-To: <1454489288-31951-2-git-send-email-jitao.shi@mediatek.com> References: <1454489288-31951-1-git-send-email-jitao.shi@mediatek.com> <1454489288-31951-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 Hi Jitao, Looks really good. Just a couple of tiny things... On Wed, Feb 3, 2016 at 4:48 PM, Jitao Shi wrote: > > This patch adds drm_bridge driver for parade DSI to eDP bridge chip. > > Signed-off-by: Jitao Shi > --- [snip] > +static int ps8640_get_modes(struct drm_connector *connector) > +{ > + struct ps8640 *ps_bridge = connector_to_ps8640(connector); > + u8 *edid; > + int ret, num_modes = 0; > + bool power_off; > + > + if (ps_bridge->edid) > + return drm_add_edid_modes(connector, ps_bridge->edid); > + > + power_off = !ps_bridge->enabled; > + ps8640_pre_enable(&ps_bridge->bridge); > + > + edid = kmalloc(EDID_LENGTH, GFP_KERNEL); devm_kmalloc() if you can. Or at least kfree() in ps8640_remove > + if (!edid) { > + DRM_ERROR("Failed to allocate EDID\n"); > + return 0; > + } > + > + ret = ps8640_read(ps_bridge->ddc_i2c, 0, edid, EDID_LENGTH); > + if (ret) { > + kfree(edid); > + goto out; > + } > + > + ps_bridge->edid = (struct edid *)edid; > + drm_mode_connector_update_edid_property(connector, ps_bridge->edid); > + > + num_modes = drm_add_edid_modes(connector, ps_bridge->edid); > + > +out: > + if (power_off) > + ps8640_post_disable(&ps_bridge->bridge); > + > + return num_modes; > +} [snip] > +static int ps8640_spi_normal_mode(struct ps8640 *ps_bridge) > +{ > + u8 cmd[2]; > + struct i2c_client *client = ps_bridge->page[2]; > + > + /* Enable-Write-Status-Register */ > + cmd[0] = WRITE_ENABLE_CMD; > + ps8640_spi_send_cmd(ps_bridge, cmd, 1); > + > + /* protect BPL/BP0/BP1 */ > + cmd[0] = WRITE_STATUS_REG_CMD; > + cmd[1] = BLK_PROTECT_BITS | STATUS_REG_PROTECT; > + ps8640_spi_send_cmd(ps_bridge, cmd, 2); > + > + /* wait for SPI rom ready */ > + ps8640_wait_rom_idle(ps_bridge); > + > + /* disable PS8640 mapping function */ > + ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, 0x00); > + > + gpiod_set_value(ps_bridge->gpio_mode_sel_n, 1); > + return 0; > +} > + > +static int ps8640_enter_bl(struct ps8640 *ps_bridge) > +{ > + ps_bridge->in_fw_update = true; > + return ps8640_spi_dl_mode(ps_bridge); On error: ps_bridge->in_fw_update = false; Thanks, -Dan