lkml.org 
[lkml]   [2017]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 1/3] drm/bridge/synopsys: dsi: stop clobbering drvdata
From
Date
Hi Brian,


On 2017年12月06日 02:56, Brian Norris wrote:
> 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:
>
Thank you for pointing out my mistake.
I will fix this in next version.

Nickey.
> 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/
>
>
>


\
 
 \ /
  Last update: 2017-12-06 08:17    [W:0.970 / U:0.448 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site