From: Archit Taneja <architt@codeaurora.org> To: John Stultz <john.stultz@linaro.org> Cc: Arnd Bergmann <arnd@arndb.de>, Laurent Pinchart <Laurent.pinchart@ideasonboard.com>, David Airlie <airlied@linux.ie>, Lars-Peter Clausen <lars@metafoo.de>, Bhumika Goyal <bhumirks@gmail.com>, Inki Dae <inki.dae@samsung.com>, dri-devel@lists.freedesktop.org, lkml <linux-kernel@vger.kernel.org>, Xinliang Liu <xinliang.liu@linaro.org>, Dan Carpenter <dan.carpenter@oracle.com>, Sean Paul <seanpaul@chromium.org>, Hans Verkuil <hans.verkuil@cisco.com>, Guodong Xu <guodong.xu@linaro.org> Subject: Re: [RFC][PATCH] drm: adv7511/33: Fix adv7511_cec_init() failure handling Date: Wed, 29 Nov 2017 10:35:54 +0530 [thread overview] Message-ID: <deab5e2a-0e49-6954-2734-ec52aa61ff3d@codeaurora.org> (raw) In-Reply-To: <CALAqxLU+jo-F_8Q7n72CP5gQO1JhRiQZaWGSn=q57v9Ofiv8fg@mail.gmail.com> On 11/29/2017 03:02 AM, John Stultz wrote: > On Sun, Nov 26, 2017 at 4:56 AM, Archit Taneja <architt@codeaurora.org> wrote: >> >> On 11/17/2017 04:29 AM, John Stultz wrote: >>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >>> index 0e14f15..939c3b9 100644 >>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >>> @@ -1203,12 +1203,12 @@ static int adv7511_probe(struct i2c_client *i2c, >>> const struct i2c_device_id *id) >>> #ifdef CONFIG_DRM_I2C_ADV7511_CEC >>> ret = adv7511_cec_init(dev, adv7511, offset); >>> - if (ret) >>> - goto err_unregister_cec; >>> #else >>> - regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset, >>> - ADV7511_CEC_CTRL_POWER_DOWN); >>> + ret = 1; >>> #endif >>> + if (ret) >>> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + >>> offset, >>> + ADV7511_CEC_CTRL_POWER_DOWN); >> >> >> This would force CEC to be powered off even if adv7511_cec_init() returned >> 0, right? > > I don't think so. The initent was its only powered off if > adv7511_cec_init returns an error or if CONFIG_DRM_I2C_ADV7511_CEC is > not set. > >> Do we know why the call to adv7511_cec_init() is failing on the Hikey board? >> If it's >> because there isn't a "cec" clock specified in DT, it's not really a fatal >> error, it >> just means that the platform hasn't been set up to support CEC. In that > > Yea. I believe this is the case w/ HiKey. I don't have deeply > detailed docs on the board so I'm not sure if we will be able to > enable that or not (Xinliang/Guodong: Do you know if its possible?). > In the meantime though, we shouldn't regress. > >> case, we >> should just power down the CEC block. So, if adv7511_cec_init() would return >> a >> -ENOENT, which we could use as a hint to power down CEC. So, maybe something >> like this?: >> >> #ifdef CONFIG_DRM_I2C_ADV7511_CEC >> ret = adv7511_cec_init(dev, adv7511, offset); >> if (ret && ret != -ENOENT) >> goto err_unregister_cec; >> #endif >> if (ret) >> regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset, >> ADV7511_CEC_CTRL_POWER_DOWN); >> >> Apart from this, we should also move adv7511_cec_init() up in the probe so >> that >> it's called before the drm_bridge is registered. > > Hans has since reworked the patch w/ a new version. You might want to > check his patches and see if they fit what your imagining. Yes, I saw Hans's patch after I wrote to you. That patch looks perfect. I'll queue that to the drm-misc repo once it's rebased on 4.15-rc1. Thanks, Archit > > thanks > -john > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
WARNING: multiple messages have this Message-ID (diff)
From: Archit Taneja <architt@codeaurora.org> To: John Stultz <john.stultz@linaro.org> Cc: Guodong Xu <guodong.xu@linaro.org>, Arnd Bergmann <arnd@arndb.de>, David Airlie <airlied@linux.ie>, lkml <linux-kernel@vger.kernel.org>, dri-devel@lists.freedesktop.org, Laurent Pinchart <Laurent.pinchart@ideasonboard.com>, Hans Verkuil <hans.verkuil@cisco.com>, Dan Carpenter <dan.carpenter@oracle.com>, Bhumika Goyal <bhumirks@gmail.com> Subject: Re: [RFC][PATCH] drm: adv7511/33: Fix adv7511_cec_init() failure handling Date: Wed, 29 Nov 2017 10:35:54 +0530 [thread overview] Message-ID: <deab5e2a-0e49-6954-2734-ec52aa61ff3d@codeaurora.org> (raw) In-Reply-To: <CALAqxLU+jo-F_8Q7n72CP5gQO1JhRiQZaWGSn=q57v9Ofiv8fg@mail.gmail.com> On 11/29/2017 03:02 AM, John Stultz wrote: > On Sun, Nov 26, 2017 at 4:56 AM, Archit Taneja <architt@codeaurora.org> wrote: >> >> On 11/17/2017 04:29 AM, John Stultz wrote: >>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >>> index 0e14f15..939c3b9 100644 >>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >>> @@ -1203,12 +1203,12 @@ static int adv7511_probe(struct i2c_client *i2c, >>> const struct i2c_device_id *id) >>> #ifdef CONFIG_DRM_I2C_ADV7511_CEC >>> ret = adv7511_cec_init(dev, adv7511, offset); >>> - if (ret) >>> - goto err_unregister_cec; >>> #else >>> - regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset, >>> - ADV7511_CEC_CTRL_POWER_DOWN); >>> + ret = 1; >>> #endif >>> + if (ret) >>> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + >>> offset, >>> + ADV7511_CEC_CTRL_POWER_DOWN); >> >> >> This would force CEC to be powered off even if adv7511_cec_init() returned >> 0, right? > > I don't think so. The initent was its only powered off if > adv7511_cec_init returns an error or if CONFIG_DRM_I2C_ADV7511_CEC is > not set. > >> Do we know why the call to adv7511_cec_init() is failing on the Hikey board? >> If it's >> because there isn't a "cec" clock specified in DT, it's not really a fatal >> error, it >> just means that the platform hasn't been set up to support CEC. In that > > Yea. I believe this is the case w/ HiKey. I don't have deeply > detailed docs on the board so I'm not sure if we will be able to > enable that or not (Xinliang/Guodong: Do you know if its possible?). > In the meantime though, we shouldn't regress. > >> case, we >> should just power down the CEC block. So, if adv7511_cec_init() would return >> a >> -ENOENT, which we could use as a hint to power down CEC. So, maybe something >> like this?: >> >> #ifdef CONFIG_DRM_I2C_ADV7511_CEC >> ret = adv7511_cec_init(dev, adv7511, offset); >> if (ret && ret != -ENOENT) >> goto err_unregister_cec; >> #endif >> if (ret) >> regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL + offset, >> ADV7511_CEC_CTRL_POWER_DOWN); >> >> Apart from this, we should also move adv7511_cec_init() up in the probe so >> that >> it's called before the drm_bridge is registered. > > Hans has since reworked the patch w/ a new version. You might want to > check his patches and see if they fit what your imagining. Yes, I saw Hans's patch after I wrote to you. That patch looks perfect. I'll queue that to the drm-misc repo once it's rebased on 4.15-rc1. Thanks, Archit > > thanks > -john > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2017-11-29 5:06 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-11-15 12:37 [PATCH] [RFT] drm: adv7511/33: fix adv7511_cec_init() failure handling Arnd Bergmann 2017-11-15 12:37 ` Arnd Bergmann 2017-11-15 12:58 ` Hans Verkuil 2017-11-15 12:58 ` Hans Verkuil 2017-11-16 16:42 ` Naresh Kamboju 2017-11-16 21:50 ` John Stultz 2017-11-16 21:50 ` John Stultz 2017-11-16 22:20 ` John Stultz 2017-11-16 22:20 ` John Stultz 2017-11-16 22:23 ` John Stultz 2017-11-16 22:23 ` John Stultz 2017-11-16 22:59 ` [RFC][PATCH] drm: adv7511/33: Fix " John Stultz 2017-11-16 22:59 ` John Stultz 2017-11-26 12:56 ` Archit Taneja 2017-11-28 21:32 ` John Stultz 2017-11-28 21:32 ` John Stultz 2017-11-29 5:05 ` Archit Taneja [this message] 2017-11-29 5:05 ` Archit Taneja 2017-11-29 8:15 ` Arnd Bergmann 2017-11-17 8:43 ` [RFC] [PATCH] " Hans Verkuil 2017-11-17 8:43 ` Hans Verkuil 2017-11-20 15:05 ` Hans Verkuil 2017-11-20 15:05 ` Hans Verkuil 2017-11-20 15:06 ` Hans Verkuil 2017-11-20 20:13 ` John Stultz 2017-11-20 20:13 ` John Stultz 2017-11-20 20:57 ` [PATCHv2] " Hans Verkuil 2017-11-20 20:57 ` Hans Verkuil 2017-11-20 23:12 ` John Stultz 2017-11-20 23:12 ` John Stultz 2017-11-21 6:48 ` Laurent Pinchart 2017-11-21 8:18 ` Hans Verkuil 2017-11-21 8:18 ` Hans Verkuil 2017-11-21 8:17 ` [PATCHv3] " Hans Verkuil 2017-11-21 8:17 ` Hans Verkuil 2017-11-21 8:22 ` Laurent Pinchart 2017-11-23 0:22 ` John Stultz 2017-11-23 0:22 ` John Stultz 2017-11-30 7:02 ` Archit Taneja 2017-11-30 7:02 ` Archit Taneja
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=deab5e2a-0e49-6954-2734-ec52aa61ff3d@codeaurora.org \ --to=architt@codeaurora.org \ --cc=Laurent.pinchart@ideasonboard.com \ --cc=airlied@linux.ie \ --cc=arnd@arndb.de \ --cc=bhumirks@gmail.com \ --cc=dan.carpenter@oracle.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=guodong.xu@linaro.org \ --cc=hans.verkuil@cisco.com \ --cc=inki.dae@samsung.com \ --cc=john.stultz@linaro.org \ --cc=lars@metafoo.de \ --cc=linux-kernel@vger.kernel.org \ --cc=seanpaul@chromium.org \ --cc=xinliang.liu@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.