From: Brian Norris <briannorris@chromium.org>
To: Nickey Yang <nickey.yang@rock-chips.com>
Cc: Philippe CORNU <philippe.cornu@st.com>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
"heiko@sntech.de" <heiko@sntech.de>,
"mark.rutland@arm.com" <mark.rutland@arm.com>,
"airlied@linux.ie" <airlied@linux.ie>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"linux-rockchip@lists.infradead.org"
<linux-rockchip@lists.infradead.org>,
"laurent.pinchart@ideasonboard.com"
<laurent.pinchart@ideasonboard.com>,
"seanpaul@chromium.org" <seanpaul@chromium.org>,
"mka@chromium.org" <mka@chromium.org>,
"hoegsberg@gmail.com" <hoegsberg@gmail.com>,
"architt@codeaurora.org" <architt@codeaurora.org>,
Yannick FERTRE <yannick.fertre@st.com>,
"hl@rock-chips.com" <hl@rock-chips.com>,
"zyw@rock-chips.com" <zyw@rock-chips.com>,
"xbl@rock-chips.com" <xbl@rock-chips.com>
Subject: Re: [PATCH v4 1/3] drm/bridge/synopsys: dsi: stop clobbering drvdata
Date: Tue, 5 Dec 2017 10:56:52 -0800 [thread overview]
Message-ID: <20171205185651.GA136446@google.com> (raw)
In-Reply-To: <206b071f-2b57-c4a2-31b3-2f469091e5e2@rock-chips.com>
Hi Nickey,
On Tue, Dec 05, 2017 at 05:14:11PM +0800, Nickey Yang wrote:
> On 2017年12月01日 18:07, Philippe CORNU wrote:
> >On 12/01/2017 10:11 AM, Nickey Yang wrote:
> >>On 2017年12月01日 16:32, Philippe CORNU wrote:
> >>>I am sorry to say that but you can not add my "Acked-by" to this patch
> >>>because this code is different from the "original" one from Brian (which
> >>>got my "Acked-by").
> >>I'm sorry I didn't think much about it, Thank you for correcting me.
> >>>Sometimes it is not an issue because differences are not important but
> >>>in this particular case, the code is really different: you have remove
> >>>platform_set_drvdata() & platform_get_drvdata() in the stm part.
> >>>
> >>>Could you please go back to the original code or propose me an updated
> >>>version of this code.
> >>Could you help update new version of this code(stm part) and then test on
> >>stm platform?
> >I think you can simply goes back to the original version from Brian (see
> >the discussion thread in https://patchwork.kernel.org/patch/10078493/)
> >unless you have specific/good reasons for modifying the code as you did.
> mmm,I'm sorry, I feel a little puzzled. Do you means we should abandon
> Brian's patch (https://patchwork.kernel.org/patch/10078493/)?
> I think we need to adjust stm part because dw_mipi_dsi_stm.c calls
> bridge's drivers if we want merge Brian's patch.
It's really simple. Your code is different from the patch I sent, and in
a way that Philippe did not like. I'll highlight it again below:
> >>>>diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> >>>>index e5b6310..80f9950 100644
> >>>>--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> >>>>+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> >>>>@@ -66,6 +66,7 @@ enum dsi_color {
> >>>> struct dw_mipi_dsi_stm {
> >>>> void __iomem *base;
> >>>> struct clk *pllref_clk;
> >>>>+ struct dw_mipi_dsi *dmd;
> >>>> };
> >>>> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
> >>>>@@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
> >>>> dw_mipi_dsi_stm_plat_data.base = dsi->base;
> >>>> dw_mipi_dsi_stm_plat_data.priv_data = dsi;
> >>>>- ret = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> >>>>- if (ret) {
> >>>>+ dsi->dmd = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> >>>>+ if (IS_ERR(dsi->dmd)) {
> >>>> DRM_ERROR("Failed to initialize mipi dsi host\n");
> >>>> clk_disable_unprepare(dsi->pllref_clk);
> >>>>+ return PTR_ERR(dsi->dmd);
> >>>> }
> >>>> return ret;
> >>>>@@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
> >>>> struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
> >>>> clk_disable_unprepare(dsi->pllref_clk);
> >>>>- dw_mipi_dsi_remove(pdev);
> >>>>+ dw_mipi_dsi_remove(dsi->dmd);
> >>>> return 0;
> >>>> }
Above is your diff for dw_mipi_dsi-stm.c. Particularly, notice that
remove() is directly referencing the static dw_mipi_dsi_stm_plat_data
struct.
If you look back at my patch [1] you'll see that you're missing hunks
like this:
static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
{
- struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
+ struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev);
clk_disable_unprepare(dsi->pllref_clk);
[...]
Brian
[1] https://patchwork.kernel.org/patch/10078493/
next prev parent reply other threads:[~2017-12-05 18:56 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-01 3:58 [PATCH v4 0/3] Update ROCKCHIP DSI driver that uses dw-mipi-dsi bridge Nickey Yang
2017-12-01 3:58 ` [PATCH v4 1/3] drm/bridge/synopsys: dsi: stop clobbering drvdata Nickey Yang
2017-12-01 8:32 ` Philippe CORNU
[not found] ` <c6a53132-1ee3-00be-d52d-218560c07975@rock-chips.com>
2017-12-01 10:07 ` Philippe CORNU
2017-12-05 9:14 ` Nickey Yang
2017-12-05 18:56 ` Brian Norris [this message]
2017-12-06 7:16 ` Nickey Yang
2017-12-01 18:29 ` Emil Velikov
2017-12-01 3:58 ` [PATCH v4 2/3] dt-bindings: display: rockchip: update DSI controller Nickey Yang
2017-12-01 18:21 ` Brian Norris
2017-12-01 3:58 ` [PATCH v4 3/3] drm/rockchip: Add ROCKCHIP DW MIPI DSI controller driver Nickey Yang
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=20171205185651.GA136446@google.com \
--to=briannorris@chromium.org \
--cc=airlied@linux.ie \
--cc=architt@codeaurora.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=heiko@sntech.de \
--cc=hl@rock-chips.com \
--cc=hoegsberg@gmail.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=mka@chromium.org \
--cc=nickey.yang@rock-chips.com \
--cc=philippe.cornu@st.com \
--cc=robh+dt@kernel.org \
--cc=seanpaul@chromium.org \
--cc=xbl@rock-chips.com \
--cc=yannick.fertre@st.com \
--cc=zyw@rock-chips.com \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).