diff mbox

[v2,4/6] drm/exynos: dsi: add support for Exynos5433 SoC

Message ID 1426666591-16103-5-git-send-email-human.hwang@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hyungwon Hwang March 18, 2015, 8:16 a.m. UTC
This patch adds support for Exynos5433. The goal is achieved by
1. Getting the address of registers from driver data
2. Getting the fixed value for registers from driver data
3. Getting different number of clocks using driver data
4. Getting max frequency of pixel clock from driver data

Signed-off-by: Donghwa Lee <dh09.lee@samsung.com>
Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
---
Changes for v2:
- change the author of "drm/exynos: dsi: add support for Exynos5433 SoC" to
Hyungwon Hwang by the previous author's will
 .../devicetree/bindings/video/exynos_dsim.txt      |  24 +-
 drivers/gpu/drm/exynos/Kconfig                     |   2 +-
 drivers/gpu/drm/exynos/exynos_drm_dsi.c            | 431 ++++++++++++++-------
 3 files changed, 313 insertions(+), 144 deletions(-)

--
1.9.1

Comments

Daniel Stone March 18, 2015, 9:52 a.m. UTC | #1
Hi,

On 18 March 2015 at 08:16, Hyungwon Hwang <human.hwang@samsung.com> wrote:
> +#define REG(dsi, reg)  ((dsi)->reg_base + dsi->driver_data->regs[(reg)])

This seems like a good change in general, but please split it up: it
makes bisection much easier if you have one patch which adds no
functionality and should have exactly the same behaviour, and then
another patch which introduces your changes.

> @@ -431,15 +579,11 @@ static unsigned long exynos_dsi_set_pll(struct exynos_dsi *dsi,
>         u16 m;
>         u32 reg;
>
> -       clk_set_rate(dsi->pll_clk, dsi->pll_clk_rate);
> -
> -       fin = clk_get_rate(dsi->pll_clk);
> -       if (!fin) {
> -               dev_err(dsi->dev, "failed to get PLL clock frequency\n");
> -               return 0;
> -       }
> -
> -       dev_dbg(dsi->dev, "PLL input frequency: %lu\n", fin);
> +       /*
> +        * The input PLL clock for MIPI DSI in Exynos5433 seems to be fixed
> +        * by OSC CLK.
> +        */
> +       fin = 24 * MHZ;

Er, is this always true on other platforms as well? Shouldn't this be
a part of the DeviceTree description?

> @@ -509,7 +656,7 @@ static int exynos_dsi_enable_clock(struct exynos_dsi *dsi)
>         dev_dbg(dsi->dev, "hs_clk = %lu, byte_clk = %lu, esc_clk = %lu\n",
>                 hs_clk, byte_clk, esc_clk);
>
> -       reg = readl(dsi->reg_base + DSIM_CLKCTRL_REG);
> +       reg = readl(REG(dsi, DSIM_CLKCTRL_REG));

Instead of this readl(REG()) pattern you have everywhere, maybe it
would be easier to introduce a dsi_read_reg(dsi, reg_enum_value)
helper, and the same for write_reg.

> @@ -1720,18 +1873,16 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>                 return -EPROBE_DEFER;
>         }
>
> -       dsi->pll_clk = devm_clk_get(dev, "pll_clk");
> -       if (IS_ERR(dsi->pll_clk)) {
> -               dev_info(dev, "failed to get dsi pll input clock\n");
> -               ret = PTR_ERR(dsi->pll_clk);
> -               goto err_del_component;
> -       }
> -
> -       dsi->bus_clk = devm_clk_get(dev, "bus_clk");
> -       if (IS_ERR(dsi->bus_clk)) {
> -               dev_info(dev, "failed to get dsi bus clock\n");
> -               ret = PTR_ERR(dsi->bus_clk);
> -               goto err_del_component;
> +       dsi->clks = devm_kzalloc(dev,
> +                               sizeof(*dsi->clks) * dsi->driver_data->num_clks,
> +                               GFP_KERNEL);
> +       for (i = 0; i < dsi->driver_data->num_clks; i++) {
> +               dsi->clks[i] = devm_clk_get(dev, clk_names[i]);
> +               if (IS_ERR(dsi->clks[i])) {
> +                       dev_info(dev, "failed to get dsi pll input clock\n");

This error message seems wrong; it should contain the name of the
actual failing clock.

Cheers,
Daniel
Hyungwon Hwang March 19, 2015, 1:02 a.m. UTC | #2
Dear Daniel,

On Wed, 18 Mar 2015 09:52:33 +0000
Daniel Stone <daniel@fooishbar.org> wrote:

> Hi,
> 
> On 18 March 2015 at 08:16, Hyungwon Hwang <human.hwang@samsung.com>
> wrote:
> > +#define REG(dsi, reg)  ((dsi)->reg_base +
> > dsi->driver_data->regs[(reg)])
> 
> This seems like a good change in general, but please split it up: it
> makes bisection much easier if you have one patch which adds no
> functionality and should have exactly the same behaviour, and then
> another patch which introduces your changes.

Yes. I agree with you.

> 
> > @@ -431,15 +579,11 @@ static unsigned long
> > exynos_dsi_set_pll(struct exynos_dsi *dsi, u16 m;
> >         u32 reg;
> >
> > -       clk_set_rate(dsi->pll_clk, dsi->pll_clk_rate);
> > -
> > -       fin = clk_get_rate(dsi->pll_clk);
> > -       if (!fin) {
> > -               dev_err(dsi->dev, "failed to get PLL clock
> > frequency\n");
> > -               return 0;
> > -       }
> > -
> > -       dev_dbg(dsi->dev, "PLL input frequency: %lu\n", fin);
> > +       /*
> > +        * The input PLL clock for MIPI DSI in Exynos5433 seems to
> > be fixed
> > +        * by OSC CLK.
> > +        */
> > +       fin = 24 * MHZ;
> 
> Er, is this always true on other platforms as well? Shouldn't this be
> a part of the DeviceTree description?

I forgot to change the comment in development. Finally it is found that
all exynos mipi dsi's fin is OSC clk which is 24 MHz. So I will remove
the comment, but remain the code as it is.

> 
> > @@ -509,7 +656,7 @@ static int exynos_dsi_enable_clock(struct
> > exynos_dsi *dsi) dev_dbg(dsi->dev, "hs_clk = %lu, byte_clk = %lu,
> > esc_clk = %lu\n", hs_clk, byte_clk, esc_clk);
> >
> > -       reg = readl(dsi->reg_base + DSIM_CLKCTRL_REG);
> > +       reg = readl(REG(dsi, DSIM_CLKCTRL_REG));
> 
> Instead of this readl(REG()) pattern you have everywhere, maybe it
> would be easier to introduce a dsi_read_reg(dsi, reg_enum_value)
> helper, and the same for write_reg.

I think that it can make the code more readable. I agree.

> 
> > @@ -1720,18 +1873,16 @@ static int exynos_dsi_probe(struct
> > platform_device *pdev) return -EPROBE_DEFER;
> >         }
> >
> > -       dsi->pll_clk = devm_clk_get(dev, "pll_clk");
> > -       if (IS_ERR(dsi->pll_clk)) {
> > -               dev_info(dev, "failed to get dsi pll input
> > clock\n");
> > -               ret = PTR_ERR(dsi->pll_clk);
> > -               goto err_del_component;
> > -       }
> > -
> > -       dsi->bus_clk = devm_clk_get(dev, "bus_clk");
> > -       if (IS_ERR(dsi->bus_clk)) {
> > -               dev_info(dev, "failed to get dsi bus clock\n");
> > -               ret = PTR_ERR(dsi->bus_clk);
> > -               goto err_del_component;
> > +       dsi->clks = devm_kzalloc(dev,
> > +                               sizeof(*dsi->clks) *
> > dsi->driver_data->num_clks,
> > +                               GFP_KERNEL);
> > +       for (i = 0; i < dsi->driver_data->num_clks; i++) {
> > +               dsi->clks[i] = devm_clk_get(dev, clk_names[i]);
> > +               if (IS_ERR(dsi->clks[i])) {
> > +                       dev_info(dev, "failed to get dsi pll input
> > clock\n");
> 
> This error message seems wrong; it should contain the name of the
> actual failing clock.

Oh. I forgot. I will change it.

Thanks for your review. I will send it again with the changes you
suggested.

> 
> Cheers,
> Daniel
Daniel Stone March 19, 2015, 1:13 a.m. UTC | #3
Hi Hyungwon,

On 19 March 2015 at 01:02, Hyungwon Hwang <human.hwang@samsung.com> wrote:
>> > +       /*
>> > +        * The input PLL clock for MIPI DSI in Exynos5433 seems to
>> > be fixed
>> > +        * by OSC CLK.
>> > +        */
>> > +       fin = 24 * MHZ;
>>
>> Er, is this always true on other platforms as well? Shouldn't this be
>> a part of the DeviceTree description?
>
> I forgot to change the comment in development. Finally it is found that
> all exynos mipi dsi's fin is OSC clk which is 24 MHz. So I will remove
> the comment, but remain the code as it is.

Fair enough. Should pll_clk be removed from the DT description then,
if it's fixed to the oscillator?

> Thanks for your review. I will send it again with the changes you
> suggested.

Thanks very much!

Cheers,
Daniel
Hyungwon Hwang March 19, 2015, 1:18 a.m. UTC | #4
Dear Daniel,

On Thu, 19 Mar 2015 01:13:21 +0000
Daniel Stone <daniel@fooishbar.org> wrote:

> Hi Hyungwon,
> 
> On 19 March 2015 at 01:02, Hyungwon Hwang <human.hwang@samsung.com>
> wrote:
> >> > +       /*
> >> > +        * The input PLL clock for MIPI DSI in Exynos5433 seems
> >> > to be fixed
> >> > +        * by OSC CLK.
> >> > +        */
> >> > +       fin = 24 * MHZ;
> >>
> >> Er, is this always true on other platforms as well? Shouldn't this
> >> be a part of the DeviceTree description?
> >
> > I forgot to change the comment in development. Finally it is found
> > that all exynos mipi dsi's fin is OSC clk which is 24 MHz. So I
> > will remove the comment, but remain the code as it is.
> 
> Fair enough. Should pll_clk be removed from the DT description then,
> if it's fixed to the oscillator?

Yes. It is redundant to represent pll_clk in DT, and it should be
removed.

> 
> > Thanks for your review. I will send it again with the changes you
> > suggested.
> 
> Thanks very much!
> 
> Cheers,
> Daniel

Best regards,
Hyungwon Hwang
Andrzej Hajda March 19, 2015, 9:18 a.m. UTC | #5
On 03/19/2015 02:18 AM, Hyungwon Hwang wrote:
> Dear Daniel,
> 
> On Thu, 19 Mar 2015 01:13:21 +0000
> Daniel Stone <daniel-rLtY4a/8tF1rovVCs/uTlw@public.gmane.org> wrote:
> 
>> Hi Hyungwon,
>>
>> On 19 March 2015 at 01:02, Hyungwon Hwang <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> wrote:
>>>>> +       /*
>>>>> +        * The input PLL clock for MIPI DSI in Exynos5433 seems
>>>>> to be fixed
>>>>> +        * by OSC CLK.
>>>>> +        */
>>>>> +       fin = 24 * MHZ;
>>>>
>>>> Er, is this always true on other platforms as well? Shouldn't this
>>>> be a part of the DeviceTree description?
>>>
>>> I forgot to change the comment in development. Finally it is found
>>> that all exynos mipi dsi's fin is OSC clk which is 24 MHz. So I
>>> will remove the comment, but remain the code as it is.
>>
>> Fair enough. Should pll_clk be removed from the DT description then,
>> if it's fixed to the oscillator?
> 
> Yes. It is redundant to represent pll_clk in DT, and it should be
> removed.

Why do you think OSC clk determines value of pll_clk?
pll_clk is mapped to SCLK_MIPI[01] or SCLK_DSIM0 gate with few dividers
and muxes above. So at least in theory it can differ from osc clk.
Additionally this gate should be enabled so you cannot just remove it
from DT.

Regards
Andrzej

> 
>>
>>> Thanks for your review. I will send it again with the changes you
>>> suggested.
>>
>> Thanks very much!
>>
>> Cheers,
>> Daniel
> 
> Best regards,
> Hyungwon Hwang
> --
> 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
>
Andrzej Hajda March 19, 2015, 9:32 a.m. UTC | #6
On 03/19/2015 02:18 AM, Hyungwon Hwang wrote:
> Dear Daniel,
> 
> On Thu, 19 Mar 2015 01:13:21 +0000
> Daniel Stone <daniel-rLtY4a/8tF1rovVCs/uTlw@public.gmane.org> wrote:
> 
>> Hi Hyungwon,
>>
>> On 19 March 2015 at 01:02, Hyungwon Hwang <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> wrote:
>>>>> +       /*
>>>>> +        * The input PLL clock for MIPI DSI in Exynos5433 seems
>>>>> to be fixed
>>>>> +        * by OSC CLK.
>>>>> +        */
>>>>> +       fin = 24 * MHZ;
>>>>
>>>> Er, is this always true on other platforms as well? Shouldn't this
>>>> be a part of the DeviceTree description?
>>>
>>> I forgot to change the comment in development. Finally it is found
>>> that all exynos mipi dsi's fin is OSC clk which is 24 MHz. So I
>>> will remove the comment, but remain the code as it is.
>>
>> Fair enough. Should pll_clk be removed from the DT description then,
>> if it's fixed to the oscillator?
> 
> Yes. It is redundant to represent pll_clk in DT, and it should be
> removed.

Why do you think OSC clk determines value of pll_clk?
pll_clk is mapped to SCLK_MIPI[01] or SCLK_DSIM0 gate with few dividers
and muxes above. So at least in theory it can differ from osc clk.
Additionally this gate should be enabled so you cannot just remove it
from DT.

Regards
Andrzej

> 
>>
>>> Thanks for your review. I will send it again with the changes you
>>> suggested.
>>
>> Thanks very much!
>>
>> Cheers,
>> Daniel
> 
> Best regards,
> Hyungwon Hwang
> --
> 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
>
Hyungwon Hwang March 20, 2015, 5:15 a.m. UTC | #7
Dear Andrej,

On Thu, 19 Mar 2015 10:32:10 +0100
Andrzej Hajda <a.hajda@samsung.com> wrote:

> On 03/19/2015 02:18 AM, Hyungwon Hwang wrote:
> > Dear Daniel,
> > 
> > On Thu, 19 Mar 2015 01:13:21 +0000
> > Daniel Stone <daniel-rLtY4a/8tF1rovVCs/uTlw@public.gmane.org> wrote:
> > 
> >> Hi Hyungwon,
> >>
> >> On 19 March 2015 at 01:02, Hyungwon Hwang
> >> <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> >>>>> +       /*
> >>>>> +        * The input PLL clock for MIPI DSI in Exynos5433 seems
> >>>>> to be fixed
> >>>>> +        * by OSC CLK.
> >>>>> +        */
> >>>>> +       fin = 24 * MHZ;
> >>>>
> >>>> Er, is this always true on other platforms as well? Shouldn't
> >>>> this be a part of the DeviceTree description?
> >>>
> >>> I forgot to change the comment in development. Finally it is found
> >>> that all exynos mipi dsi's fin is OSC clk which is 24 MHz. So I
> >>> will remove the comment, but remain the code as it is.
> >>
> >> Fair enough. Should pll_clk be removed from the DT description
> >> then, if it's fixed to the oscillator?
> > 
> > Yes. It is redundant to represent pll_clk in DT, and it should be
> > removed.
> 
> Why do you think OSC clk determines value of pll_clk?
> pll_clk is mapped to SCLK_MIPI[01] or SCLK_DSIM0 gate with few
> dividers and muxes above. So at least in theory it can differ from
> osc clk. Additionally this gate should be enabled so you cannot just
> remove it from DT.
> 
> Regards
> Andrzej

As I found, pll clk is not SCLK_MIPI[01] but OSC CLK. SCLK_DSIM0 must
be controlled in this driver as it has been, as a gate clock of MIPI
DSI block, but not as a pll clk. SCLK_DSIM0 is not the input
clock of MIPI DPHY which provides fin in this code. So clock setting
and getting code was wrong, and must be removed.

OSC CLK is not soc-depedendant but board-dependant, even though I have
not seen any board which does not use OSC CLK by 24 MHz. It must be
parsed from board DT file, which in this case, we can use the value in
pll_clk_rate (the variable name must be renamed also).

Because ambiguous description in the technical document, I can be
wrong. Please let me know if I do not understand something. Thanks for
your comment.

Best regards,
Hyungwon Hwang

> 
> > 
> >>
> >>> Thanks for your review. I will send it again with the changes you
> >>> suggested.
> >>
> >> Thanks very much!
> >>
> >> Cheers,
> >> Daniel
> > 
> > Best regards,
> > Hyungwon Hwang
> > --
> > 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
> > 
>
Andrzej Hajda March 23, 2015, 9:31 a.m. UTC | #8
On 03/20/2015 06:15 AM, Hyungwon Hwang wrote:
> Dear Andrej,
>
> On Thu, 19 Mar 2015 10:32:10 +0100
> Andrzej Hajda <a.hajda@samsung.com> wrote:
>
>> On 03/19/2015 02:18 AM, Hyungwon Hwang wrote:
>>> Dear Daniel,
>>>
>>> On Thu, 19 Mar 2015 01:13:21 +0000
>>> Daniel Stone <daniel-rLtY4a/8tF1rovVCs/uTlw@public.gmane.org> wrote:
>>>
>>>> Hi Hyungwon,
>>>>
>>>> On 19 March 2015 at 01:02, Hyungwon Hwang
>>>> <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>>>>>>> +       /*
>>>>>>> +        * The input PLL clock for MIPI DSI in Exynos5433 seems
>>>>>>> to be fixed
>>>>>>> +        * by OSC CLK.
>>>>>>> +        */
>>>>>>> +       fin = 24 * MHZ;
>>>>>> Er, is this always true on other platforms as well? Shouldn't
>>>>>> this be a part of the DeviceTree description?
>>>>> I forgot to change the comment in development. Finally it is found
>>>>> that all exynos mipi dsi's fin is OSC clk which is 24 MHz. So I
>>>>> will remove the comment, but remain the code as it is.
>>>> Fair enough. Should pll_clk be removed from the DT description
>>>> then, if it's fixed to the oscillator?
>>> Yes. It is redundant to represent pll_clk in DT, and it should be
>>> removed.
>> Why do you think OSC clk determines value of pll_clk?
>> pll_clk is mapped to SCLK_MIPI[01] or SCLK_DSIM0 gate with few
>> dividers and muxes above. So at least in theory it can differ from
>> osc clk. Additionally this gate should be enabled so you cannot just
>> remove it from DT.
>>
>> Regards
>> Andrzej
> As I found, pll clk is not SCLK_MIPI[01] but OSC CLK. SCLK_DSIM0 must
> be controlled in this driver as it has been, as a gate clock of MIPI
> DSI block, but not as a pll clk. SCLK_DSIM0 is not the input
> clock of MIPI DPHY which provides fin in this code. So clock setting
> and getting code was wrong, and must be removed.
>
> OSC CLK is not soc-depedendant but board-dependant, even though I have
> not seen any board which does not use OSC CLK by 24 MHz. It must be
> parsed from board DT file, which in this case, we can use the value in
> pll_clk_rate (the variable name must be renamed also).
>
> Because ambiguous description in the technical document, I can be
> wrong. Please let me know if I do not understand something. Thanks for
> your comment.

After some digging I agree that documentation is quite confusing and
current code could be wrong. Anyway I wonder if it wouldn't be better
to explicitly provide input clock for DSIM, or more precisely for its PLL
instead of hardcoding 24MHz into the driver.

Another thing that bothers me is relation of DPHY_PLL in clock controller to
MIPI_DPHY in Exynos7420. There are two clocks used by MIPI_DPHY:
- "Ref Clock" pinned to SCLK_MIPIDPHY_M4 connected to OSCCLK,
- "PHY Clock" pinned to I_FOUT_DPHY_PLL connected to DPHY_PLL,

The first clock seems to be your osc clock, but what is the role of the
2nd one?

Regards
Andrzej

> Best regards,
> Hyungwon Hwang
>
>>>>> Thanks for your review. I will send it again with the changes you
>>>>> suggested.
>>>> Thanks very much!
>>>>
>>>> Cheers,
>>>> Daniel
>>> Best regards,
>>> Hyungwon Hwang
>>> --
>>> 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
>>>
Hyungwon Hwang March 23, 2015, 1:42 p.m. UTC | #9
Dear Andrej,

On Mon, 23 Mar 2015 10:31:58 +0100
Andrzej Hajda <a.hajda@samsung.com> wrote:

> On 03/20/2015 06:15 AM, Hyungwon Hwang wrote:
> > Dear Andrej,
> >
> > On Thu, 19 Mar 2015 10:32:10 +0100
> > Andrzej Hajda <a.hajda@samsung.com> wrote:
> >
> >> On 03/19/2015 02:18 AM, Hyungwon Hwang wrote:
> >>> Dear Daniel,
> >>>
> >>> On Thu, 19 Mar 2015 01:13:21 +0000
> >>> Daniel Stone <daniel-rLtY4a/8tF1rovVCs/uTlw@public.gmane.org>
> >>> wrote:
> >>>
> >>>> Hi Hyungwon,
> >>>>
> >>>> On 19 March 2015 at 01:02, Hyungwon Hwang
> >>>> <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> >>>>>>> +       /*
> >>>>>>> +        * The input PLL clock for MIPI DSI in Exynos5433
> >>>>>>> seems to be fixed
> >>>>>>> +        * by OSC CLK.
> >>>>>>> +        */
> >>>>>>> +       fin = 24 * MHZ;
> >>>>>> Er, is this always true on other platforms as well? Shouldn't
> >>>>>> this be a part of the DeviceTree description?
> >>>>> I forgot to change the comment in development. Finally it is
> >>>>> found that all exynos mipi dsi's fin is OSC clk which is 24
> >>>>> MHz. So I will remove the comment, but remain the code as it is.
> >>>> Fair enough. Should pll_clk be removed from the DT description
> >>>> then, if it's fixed to the oscillator?
> >>> Yes. It is redundant to represent pll_clk in DT, and it should be
> >>> removed.
> >> Why do you think OSC clk determines value of pll_clk?
> >> pll_clk is mapped to SCLK_MIPI[01] or SCLK_DSIM0 gate with few
> >> dividers and muxes above. So at least in theory it can differ from
> >> osc clk. Additionally this gate should be enabled so you cannot
> >> just remove it from DT.
> >>
> >> Regards
> >> Andrzej
> > As I found, pll clk is not SCLK_MIPI[01] but OSC CLK. SCLK_DSIM0
> > must be controlled in this driver as it has been, as a gate clock
> > of MIPI DSI block, but not as a pll clk. SCLK_DSIM0 is not the input
> > clock of MIPI DPHY which provides fin in this code. So clock setting
> > and getting code was wrong, and must be removed.
> >
> > OSC CLK is not soc-depedendant but board-dependant, even though I
> > have not seen any board which does not use OSC CLK by 24 MHz. It
> > must be parsed from board DT file, which in this case, we can use
> > the value in pll_clk_rate (the variable name must be renamed also).
> >
> > Because ambiguous description in the technical document, I can be
> > wrong. Please let me know if I do not understand something. Thanks
> > for your comment.
> 
> After some digging I agree that documentation is quite confusing and
> current code could be wrong. Anyway I wonder if it wouldn't be better
> to explicitly provide input clock for DSIM, or more precisely for its
> PLL instead of hardcoding 24MHz into the driver.

OK. I agree. It will be more explicit to get the clock rate from DT.

> 
> Another thing that bothers me is relation of DPHY_PLL in clock
> controller to MIPI_DPHY in Exynos7420. There are two clocks used by
> MIPI_DPHY:
> - "Ref Clock" pinned to SCLK_MIPIDPHY_M4 connected to OSCCLK,
> - "PHY Clock" pinned to I_FOUT_DPHY_PLL connected to DPHY_PLL,
> 
> The first clock seems to be your osc clock, but what is the role of
> the 2nd one?

Hmm, I couldn't find similar clock in Exynos5433, also I don't have
the manual for Exynos7420.

Best regards,
Hyungwon Hwang


> 
> Regards
> Andrzej
> 
> > Best regards,
> > Hyungwon Hwang
> >
> >>>>> Thanks for your review. I will send it again with the changes
> >>>>> you suggested.
> >>>> Thanks very much!
> >>>>
> >>>> Cheers,
> >>>> Daniel
> >>> Best regards,
> >>> Hyungwon Hwang
> >>> --
> >>> 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
> >>>
>
Hyungwon Hwang March 26, 2015, 10:33 a.m. UTC | #10
Dear Daniel,

On Wed, 18 Mar 2015 09:52:33 +0000
Daniel Stone <daniel@fooishbar.org> wrote:

> Hi,
> 
> On 18 March 2015 at 08:16, Hyungwon Hwang <human.hwang@samsung.com>
> wrote:
> > +#define REG(dsi, reg)  ((dsi)->reg_base +
> > dsi->driver_data->regs[(reg)])
> 
> This seems like a good change in general, but please split it up: it
> makes bisection much easier if you have one patch which adds no
> functionality and should have exactly the same behaviour, and then
> another patch which introduces your changes.
> 

Yes. That also looks good to me.

> > @@ -431,15 +579,11 @@ static unsigned long
> > exynos_dsi_set_pll(struct exynos_dsi *dsi, u16 m;
> >         u32 reg;
> >
> > -       clk_set_rate(dsi->pll_clk, dsi->pll_clk_rate);
> > -
> > -       fin = clk_get_rate(dsi->pll_clk);
> > -       if (!fin) {
> > -               dev_err(dsi->dev, "failed to get PLL clock
> > frequency\n");
> > -               return 0;
> > -       }
> > -
> > -       dev_dbg(dsi->dev, "PLL input frequency: %lu\n", fin);
> > +       /*
> > +        * The input PLL clock for MIPI DSI in Exynos5433 seems to
> > be fixed
> > +        * by OSC CLK.
> > +        */
> > +       fin = 24 * MHZ;
> 
> Er, is this always true on other platforms as well? Shouldn't this be
> a part of the DeviceTree description?
> 
> > @@ -509,7 +656,7 @@ static int exynos_dsi_enable_clock(struct
> > exynos_dsi *dsi) dev_dbg(dsi->dev, "hs_clk = %lu, byte_clk = %lu,
> > esc_clk = %lu\n", hs_clk, byte_clk, esc_clk);
> >
> > -       reg = readl(dsi->reg_base + DSIM_CLKCTRL_REG);
> > +       reg = readl(REG(dsi, DSIM_CLKCTRL_REG));
> 
> Instead of this readl(REG()) pattern you have everywhere, maybe it
> would be easier to introduce a dsi_read_reg(dsi, reg_enum_value)
> helper, and the same for write_reg.
> 

Yes. That's resonable.

> > @@ -1720,18 +1873,16 @@ static int exynos_dsi_probe(struct
> > platform_device *pdev) return -EPROBE_DEFER;
> >         }
> >
> > -       dsi->pll_clk = devm_clk_get(dev, "pll_clk");
> > -       if (IS_ERR(dsi->pll_clk)) {
> > -               dev_info(dev, "failed to get dsi pll input
> > clock\n");
> > -               ret = PTR_ERR(dsi->pll_clk);
> > -               goto err_del_component;
> > -       }
> > -
> > -       dsi->bus_clk = devm_clk_get(dev, "bus_clk");
> > -       if (IS_ERR(dsi->bus_clk)) {
> > -               dev_info(dev, "failed to get dsi bus clock\n");
> > -               ret = PTR_ERR(dsi->bus_clk);
> > -               goto err_del_component;
> > +       dsi->clks = devm_kzalloc(dev,
> > +                               sizeof(*dsi->clks) *
> > dsi->driver_data->num_clks,
> > +                               GFP_KERNEL);
> > +       for (i = 0; i < dsi->driver_data->num_clks; i++) {
> > +               dsi->clks[i] = devm_clk_get(dev, clk_names[i]);
> > +               if (IS_ERR(dsi->clks[i])) {
> > +                       dev_info(dev, "failed to get dsi pll input
> > clock\n");
> 
> This error message seems wrong; it should contain the name of the
> actual failing clock.
> 

OK.

Best regards,
Hyungwon Hwang

> Cheers,
> Daniel
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/video/exynos_dsim.txt b/Documentation/devicetree/bindings/video/exynos_dsim.txt
index ca2b4aa..fea7718 100644
--- a/Documentation/devicetree/bindings/video/exynos_dsim.txt
+++ b/Documentation/devicetree/bindings/video/exynos_dsim.txt
@@ -6,6 +6,7 @@  Required properties:
 		"samsung,exynos4210-mipi-dsi" /* for Exynos4 SoCs */
 		"samsung,exynos4415-mipi-dsi" /* for Exynos4415 SoC */
 		"samsung,exynos5410-mipi-dsi" /* for Exynos5410/5420/5440 SoCs */
+		"samsung,exynos5433-mipi-dsi" /* for Exynos5433 SoCs */
   - reg: physical base address and length of the registers set for the device
   - interrupts: should contain DSI interrupt
   - clocks: list of clock specifiers, must contain an entry for each required
@@ -30,10 +31,19 @@  Video interfaces:
   Device node can contain video interface port nodes according to [2].
   The following are properties specific to those nodes:

-  port node:
-    - reg: (required) can be 0 for input RGB/I80 port or 1 for DSI port;
+  port node inbound:
+    - reg: (required) must be 0.
+  port node outbound:
+    - reg: (required) must be 1.

-  endpoint node of DSI port (reg = 1):
+  endpoint node connected from mic node (reg = 0):
+    - remote-endpoint: specifies the endpoint in mic node. This node is required
+		       for Exynos5433 mipi dsi. So mic can access to panel node
+		       thoughout this dsi node.
+  endpoint node connected to panel node (reg = 1):
+    - remote-endpoint: specifies the endpoint in panel node. This node is
+		       required in all kinds of exynos mipi dsi to represent
+		       the connection between mipi dsi and panel.
     - samsung,burst-clock-frequency: specifies DSI frequency in high-speed burst
       mode
     - samsung,esc-clock-frequency: specifies DSI frequency in escape mode
@@ -72,7 +82,15 @@  Example:
 			#address-cells = <1>;
 			#size-cells = <0>;

+			port@0 {
+				reg = <0>;
+				decon_to_mic: endpoint {
+					remote-endpoint = <&mic_to_decon>;
+				};
+			};
+
 			port@1 {
+				reg = <1>;
 				dsi_ep: endpoint {
 					reg = <0>;
 					samsung,burst-clock-frequency = <500000000>;
diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index a796175..eb35a21 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -47,7 +47,7 @@  config DRM_EXYNOS_DPI

 config DRM_EXYNOS_DSI
 	bool "EXYNOS DRM MIPI-DSI driver support"
-	depends on (DRM_EXYNOS_FIMD || DRM_EXYNOS7_DECON)
+	depends on (DRM_EXYNOS_FIMD || DRM_EXYNOS5433_DECON || DRM_EXYNOS7_DECON)
 	select DRM_MIPI_DSI
 	select DRM_PANEL
 	default n
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 05fe93d..77236ad 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -33,38 +33,6 @@ 
 /* returns true iff both arguments logically differs */
 #define NEQV(a, b) (!(a) ^ !(b))

-#define DSIM_STATUS_REG		0x0	/* Status register */
-#define DSIM_SWRST_REG		0x4	/* Software reset register */
-#define DSIM_CLKCTRL_REG	0x8	/* Clock control register */
-#define DSIM_TIMEOUT_REG	0xc	/* Time out register */
-#define DSIM_CONFIG_REG		0x10	/* Configuration register */
-#define DSIM_ESCMODE_REG	0x14	/* Escape mode register */
-
-/* Main display image resolution register */
-#define DSIM_MDRESOL_REG	0x18
-#define DSIM_MVPORCH_REG	0x1c	/* Main display Vporch register */
-#define DSIM_MHPORCH_REG	0x20	/* Main display Hporch register */
-#define DSIM_MSYNC_REG		0x24	/* Main display sync area register */
-
-/* Sub display image resolution register */
-#define DSIM_SDRESOL_REG	0x28
-#define DSIM_INTSRC_REG		0x2c	/* Interrupt source register */
-#define DSIM_INTMSK_REG		0x30	/* Interrupt mask register */
-#define DSIM_PKTHDR_REG		0x34	/* Packet Header FIFO register */
-#define DSIM_PAYLOAD_REG	0x38	/* Payload FIFO register */
-#define DSIM_RXFIFO_REG		0x3c	/* Read FIFO register */
-#define DSIM_FIFOTHLD_REG	0x40	/* FIFO threshold level register */
-#define DSIM_FIFOCTRL_REG	0x44	/* FIFO status and control register */
-
-/* FIFO memory AC characteristic register */
-#define DSIM_PLLCTRL_REG	0x4c	/* PLL control register */
-#define DSIM_PHYACCHR_REG	0x54	/* D-PHY AC characteristic register */
-#define DSIM_PHYACCHR1_REG	0x58	/* D-PHY AC characteristic register1 */
-#define DSIM_PHYCTRL_REG	0x5c
-#define DSIM_PHYTIMING_REG	0x64
-#define DSIM_PHYTIMING1_REG	0x68
-#define DSIM_PHYTIMING2_REG	0x6c
-
 /* DSIM_STATUS */
 #define DSIM_STOP_STATE_DAT(x)		(((x) & 0xf) << 0)
 #define DSIM_STOP_STATE_CLK		(1 << 8)
@@ -128,8 +96,8 @@ 

 /* DSIM_MDRESOL */
 #define DSIM_MAIN_STAND_BY		(1 << 31)
-#define DSIM_MAIN_VRESOL(x)		(((x) & 0x7ff) << 16)
-#define DSIM_MAIN_HRESOL(x)		(((x) & 0X7ff) << 0)
+#define DSIM_MAIN_VRESOL(x, num_bits)	(((x) & ((1 << (num_bits)) - 1)) << 16)
+#define DSIM_MAIN_HRESOL(x, num_bits)	(((x) & ((1 << (num_bits)) - 1)) << 0)

 /* DSIM_MVPORCH */
 #define DSIM_CMD_ALLOW(x)		((x) << 28)
@@ -163,6 +131,7 @@ 
 #define DSIM_INT_PLL_STABLE		(1 << 31)
 #define DSIM_INT_SW_RST_RELEASE		(1 << 30)
 #define DSIM_INT_SFR_FIFO_EMPTY		(1 << 29)
+#define DSIM_INT_SFR_HDR_FIFO_EMPTY	(1 << 28)
 #define DSIM_INT_BTA			(1 << 25)
 #define DSIM_INT_FRAME_DONE		(1 << 24)
 #define DSIM_INT_RX_TIMEOUT		(1 << 21)
@@ -211,6 +180,8 @@ 

 /* DSIM_PHYCTRL */
 #define DSIM_PHYCTRL_ULPS_EXIT(x)	(((x) & 0x1ff) << 0)
+#define DSIM_PHYCTRL_B_DPHYCTL_VREG_LP	(1 << 30)
+#define DSIM_PHYCTRL_B_DPHYCTL_SLEW_UP	(1 << 14)

 /* DSIM_PHYTIMING */
 #define DSIM_PHYTIMING_LPX(x)		((x) << 8)
@@ -234,6 +205,12 @@ 
 #define DSI_XFER_TIMEOUT_MS		100
 #define DSI_RX_FIFO_EMPTY		0x30800002

+#define REG(dsi, reg)	((dsi)->reg_base + dsi->driver_data->regs[(reg)])
+
+static char *clk_names[5] = { "bus_clk", "pll_clk",
+	"phyclk_mipidphy0_bitclkdiv8", "phyclk_mipidphy0_rxclkesc0",
+	"sclk_rgb_vclk_to_dsim0" };
+
 enum exynos_dsi_transfer_type {
 	EXYNOS_DSI_TX,
 	EXYNOS_DSI_RX,
@@ -261,10 +238,15 @@  struct exynos_dsi_transfer {
 #define DSIM_STATE_CMD_LPM		BIT(2)

 struct exynos_dsi_driver_data {
+	unsigned int *regs;
 	unsigned int plltmr_reg;
-
 	unsigned int has_freqband:1;
 	unsigned int has_clklane_stop:1;
+	unsigned int num_clks;
+	unsigned int max_freq;
+	unsigned int wait_for_reset;
+	unsigned int num_bits_resol;
+	unsigned int *values;
 };

 struct exynos_dsi {
@@ -277,8 +259,7 @@  struct exynos_dsi {

 	void __iomem *reg_base;
 	struct phy *phy;
-	struct clk *pll_clk;
-	struct clk *bus_clk;
+	struct clk **clks;
 	struct regulator_bulk_data supplies[2];
 	int irq;
 	int te_gpio;
@@ -309,25 +290,186 @@  static inline struct exynos_dsi *display_to_dsi(struct exynos_drm_display *d)
 	return container_of(d, struct exynos_dsi, display);
 }

+enum regs {
+	DSIM_STATUS_REG,	/* Status register */
+	DSIM_SWRST_REG,		/* Software reset register */
+	DSIM_CLKCTRL_REG,	/* Clock control register */
+	DSIM_TIMEOUT_REG,	/* Time out register */
+	DSIM_CONFIG_REG,	/* Configuration register */
+	DSIM_ESCMODE_REG,	/* Escape mode register */
+	DSIM_MDRESOL_REG,
+	DSIM_MVPORCH_REG,	/* Main display Vporch register */
+	DSIM_MHPORCH_REG,	/* Main display Hporch register */
+	DSIM_MSYNC_REG,		/* Main display sync area register */
+	DSIM_INTSRC_REG,	/* Interrupt source register */
+	DSIM_INTMSK_REG,	/* Interrupt mask register */
+	DSIM_PKTHDR_REG,	/* Packet Header FIFO register */
+	DSIM_PAYLOAD_REG,	/* Payload FIFO register */
+	DSIM_RXFIFO_REG,	/* Read FIFO register */
+	DSIM_FIFOCTRL_REG,	/* FIFO status and control register */
+	DSIM_PLLCTRL_REG,	/* PLL control register */
+	DSIM_PHYCTRL_REG,
+	DSIM_PHYTIMING_REG,
+	DSIM_PHYTIMING1_REG,
+	DSIM_PHYTIMING2_REG,
+	NUM_REGS
+};
+static unsigned int regs[] = {
+	[DSIM_STATUS_REG] =  0x00,
+	[DSIM_SWRST_REG] =  0x04,
+	[DSIM_CLKCTRL_REG] =  0x08,
+	[DSIM_TIMEOUT_REG] =  0x0c,
+	[DSIM_CONFIG_REG] =  0x10,
+	[DSIM_ESCMODE_REG] =  0x14,
+	[DSIM_MDRESOL_REG] =  0x18,
+	[DSIM_MVPORCH_REG] =  0x1c,
+	[DSIM_MHPORCH_REG] =  0x20,
+	[DSIM_MSYNC_REG] =  0x24,
+	[DSIM_INTSRC_REG] =  0x2c,
+	[DSIM_INTMSK_REG] =  0x30,
+	[DSIM_PKTHDR_REG] =  0x34,
+	[DSIM_PAYLOAD_REG] =  0x38,
+	[DSIM_RXFIFO_REG] =  0x3c,
+	[DSIM_FIFOCTRL_REG] =  0x44,
+	[DSIM_PLLCTRL_REG] =  0x4c,
+	[DSIM_PHYCTRL_REG] =  0x5c,
+	[DSIM_PHYTIMING_REG] =  0x64,
+	[DSIM_PHYTIMING1_REG] =  0x68,
+	[DSIM_PHYTIMING2_REG] =  0x6c,
+};
+
+static unsigned int exynos5433_regs[] = {
+	[DSIM_STATUS_REG] = 0x04,
+	[DSIM_SWRST_REG] = 0x0C,
+	[DSIM_CLKCTRL_REG] = 0x10,
+	[DSIM_TIMEOUT_REG] = 0x14,
+	[DSIM_CONFIG_REG] = 0x18,
+	[DSIM_ESCMODE_REG] = 0x1C,
+	[DSIM_MDRESOL_REG] = 0x20,
+	[DSIM_MVPORCH_REG] = 0x24,
+	[DSIM_MHPORCH_REG] = 0x28,
+	[DSIM_MSYNC_REG] = 0x2C,
+	[DSIM_INTSRC_REG] = 0x34,
+	[DSIM_INTMSK_REG] = 0x38,
+	[DSIM_PKTHDR_REG] = 0x3C,
+	[DSIM_PAYLOAD_REG] = 0x40,
+	[DSIM_RXFIFO_REG] = 0x44,
+	[DSIM_FIFOCTRL_REG] = 0x4C,
+	[DSIM_PLLCTRL_REG] = 0x94,
+	[DSIM_PHYCTRL_REG] = 0xA4,
+	[DSIM_PHYTIMING_REG] = 0xB4,
+	[DSIM_PHYTIMING1_REG] = 0xB8,
+	[DSIM_PHYTIMING2_REG] = 0xBC,
+};
+
+enum values {
+	RESET_TYPE,
+	PLL_TIMER,
+	STOP_STATE_CNT,
+	PHYCTRL_ULPS_EXIT,
+	PHYCTRL_VREG_LP,
+	PHYCTRL_SLEW_UP,
+	PHYTIMING_LPX,
+	PHYTIMING_HS_EXIT,
+	PHYTIMING_CLK_PREPARE,
+	PHYTIMING_CLK_ZERO,
+	PHYTIMING_CLK_POST,
+	PHYTIMING_CLK_TRAIL,
+	PHYTIMING_HS_PREPARE,
+	PHYTIMING_HS_ZERO,
+	PHYTIMING_HS_TRAIL
+};
+
+static unsigned int values[] = {
+	[RESET_TYPE] = DSIM_SWRST,
+	[PLL_TIMER] = 500,
+	[STOP_STATE_CNT] = 0xf,
+	[PHYCTRL_ULPS_EXIT] = DSIM_PHYCTRL_ULPS_EXIT(0x0af),
+	[PHYCTRL_VREG_LP] = 0,
+	[PHYCTRL_SLEW_UP] = 0,
+	[PHYTIMING_LPX] = DSIM_PHYTIMING_LPX(0x06),
+	[PHYTIMING_HS_EXIT] = DSIM_PHYTIMING_HS_EXIT(0x0b),
+	[PHYTIMING_CLK_PREPARE] = DSIM_PHYTIMING1_CLK_PREPARE(0x07),
+	[PHYTIMING_CLK_ZERO] = DSIM_PHYTIMING1_CLK_ZERO(0x27),
+	[PHYTIMING_CLK_POST] = DSIM_PHYTIMING1_CLK_POST(0x0d),
+	[PHYTIMING_CLK_TRAIL] = DSIM_PHYTIMING1_CLK_TRAIL(0x08),
+	[PHYTIMING_HS_PREPARE] = DSIM_PHYTIMING2_HS_PREPARE(0x09),
+	[PHYTIMING_HS_ZERO] = DSIM_PHYTIMING2_HS_ZERO(0x0d),
+	[PHYTIMING_HS_TRAIL] = DSIM_PHYTIMING2_HS_TRAIL(0x0b),
+};
+
+static unsigned int exynos5433_values[] = {
+	[RESET_TYPE] = DSIM_FUNCRST,
+	[PLL_TIMER] = 22200,
+	[STOP_STATE_CNT] = 0xa,
+	[PHYCTRL_ULPS_EXIT] = DSIM_PHYCTRL_ULPS_EXIT(0x190),
+	[PHYCTRL_VREG_LP] = DSIM_PHYCTRL_B_DPHYCTL_VREG_LP,
+	[PHYCTRL_SLEW_UP] = DSIM_PHYCTRL_B_DPHYCTL_SLEW_UP,
+	[PHYTIMING_LPX] = DSIM_PHYTIMING_LPX(0x07),
+	[PHYTIMING_HS_EXIT] = DSIM_PHYTIMING_HS_EXIT(0x0c),
+	[PHYTIMING_CLK_PREPARE] = DSIM_PHYTIMING1_CLK_PREPARE(0x09),
+	[PHYTIMING_CLK_ZERO] = DSIM_PHYTIMING1_CLK_ZERO(0x2d),
+	[PHYTIMING_CLK_POST] = DSIM_PHYTIMING1_CLK_POST(0x0e),
+	[PHYTIMING_CLK_TRAIL] = DSIM_PHYTIMING1_CLK_TRAIL(0x09),
+	[PHYTIMING_HS_PREPARE] = DSIM_PHYTIMING2_HS_PREPARE(0x0b),
+	[PHYTIMING_HS_ZERO] = DSIM_PHYTIMING2_HS_ZERO(0x10),
+	[PHYTIMING_HS_TRAIL] = DSIM_PHYTIMING2_HS_TRAIL(0x0c),
+};
+
 static struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
+	.regs = regs,
 	.plltmr_reg = 0x50,
 	.has_freqband = 1,
 	.has_clklane_stop = 1,
+	.num_clks = 2,
+	.max_freq = 1000,
+	.wait_for_reset = 1,
+	.num_bits_resol = 11,
+	.values = values,
 };

 static struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
+	.regs = regs,
 	.plltmr_reg = 0x50,
 	.has_freqband = 1,
 	.has_clklane_stop = 1,
+	.num_clks = 2,
+	.max_freq = 1000,
+	.wait_for_reset = 1,
+	.num_bits_resol = 11,
+	.values = values,
 };

 static struct exynos_dsi_driver_data exynos4415_dsi_driver_data = {
+	.regs = regs,
 	.plltmr_reg = 0x58,
 	.has_clklane_stop = 1,
+	.num_clks = 2,
+	.max_freq = 1000,
+	.wait_for_reset = 1,
+	.num_bits_resol = 11,
+	.values = values,
 };

 static struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
+	.regs = regs,
 	.plltmr_reg = 0x58,
+	.num_clks = 2,
+	.max_freq = 1000,
+	.wait_for_reset = 1,
+	.num_bits_resol = 11,
+	.values = values,
+};
+
+static struct exynos_dsi_driver_data exynos5433_dsi_driver_data = {
+	.regs = exynos5433_regs,
+	.plltmr_reg = 0xa0,
+	.has_clklane_stop = 1,
+	.num_clks = 5,
+	.max_freq = 1500,
+	.wait_for_reset = 0,
+	.num_bits_resol = 12,
+	.values = exynos5433_values,
 };

 static struct of_device_id exynos_dsi_of_match[] = {
@@ -339,6 +481,8 @@  static struct of_device_id exynos_dsi_of_match[] = {
 	  .data = &exynos4415_dsi_driver_data },
 	{ .compatible = "samsung,exynos5410-mipi-dsi",
 	  .data = &exynos5_dsi_driver_data },
+	{ .compatible = "samsung,exynos5433-mipi-dsi",
+	  .data = &exynos5433_dsi_driver_data },
 	{ }
 };

@@ -361,8 +505,10 @@  static void exynos_dsi_wait_for_reset(struct exynos_dsi *dsi)

 static void exynos_dsi_reset(struct exynos_dsi *dsi)
 {
+	struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
+
 	reinit_completion(&dsi->completed);
-	writel(DSIM_SWRST, dsi->reg_base + DSIM_SWRST_REG);
+	writel(driver_data->values[RESET_TYPE], REG(dsi, DSIM_SWRST_REG));
 }

 #ifndef MHZ
@@ -372,6 +518,7 @@  static void exynos_dsi_reset(struct exynos_dsi *dsi)
 static unsigned long exynos_dsi_pll_find_pms(struct exynos_dsi *dsi,
 		unsigned long fin, unsigned long fout, u8 *p, u16 *m, u8 *s)
 {
+	struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
 	unsigned long best_freq = 0;
 	u32 min_delta = 0xffffffff;
 	u8 p_min, p_max;
@@ -395,7 +542,8 @@  static unsigned long exynos_dsi_pll_find_pms(struct exynos_dsi *dsi,

 			tmp = (u64)_m * fin;
 			do_div(tmp, _p);
-			if (tmp < 500 * MHZ || tmp > 1000 * MHZ)
+			if (tmp < 500 * MHZ ||
+					tmp > driver_data->max_freq * MHZ)
 				continue;

 			tmp = (u64)_m * fin;
@@ -431,15 +579,11 @@  static unsigned long exynos_dsi_set_pll(struct exynos_dsi *dsi,
 	u16 m;
 	u32 reg;

-	clk_set_rate(dsi->pll_clk, dsi->pll_clk_rate);
-
-	fin = clk_get_rate(dsi->pll_clk);
-	if (!fin) {
-		dev_err(dsi->dev, "failed to get PLL clock frequency\n");
-		return 0;
-	}
-
-	dev_dbg(dsi->dev, "PLL input frequency: %lu\n", fin);
+	/*
+	 * The input PLL clock for MIPI DSI in Exynos5433 seems to be fixed
+	 * by OSC CLK.
+	 */
+	fin = 24 * MHZ;

 	fout = exynos_dsi_pll_find_pms(dsi, fin, freq, &p, &m, &s);
 	if (!fout) {
@@ -449,7 +593,8 @@  static unsigned long exynos_dsi_set_pll(struct exynos_dsi *dsi,
 	}
 	dev_dbg(dsi->dev, "PLL freq %lu, (p %d, m %d, s %d)\n", fout, p, m, s);

-	writel(500, dsi->reg_base + driver_data->plltmr_reg);
+	writel(driver_data->values[PLL_TIMER],
+			dsi->reg_base + driver_data->plltmr_reg);

 	reg = DSIM_PLL_EN | DSIM_PLL_P(p) | DSIM_PLL_M(m) | DSIM_PLL_S(s);

@@ -471,7 +616,7 @@  static unsigned long exynos_dsi_set_pll(struct exynos_dsi *dsi,
 		reg |= DSIM_FREQ_BAND(band);
 	}

-	writel(reg, dsi->reg_base + DSIM_PLLCTRL_REG);
+	writel(reg, REG(dsi, DSIM_PLLCTRL_REG));

 	timeout = 1000;
 	do {
@@ -479,7 +624,7 @@  static unsigned long exynos_dsi_set_pll(struct exynos_dsi *dsi,
 			dev_err(dsi->dev, "PLL failed to stabilize\n");
 			return 0;
 		}
-		reg = readl(dsi->reg_base + DSIM_STATUS_REG);
+		reg = readl(REG(dsi, DSIM_STATUS_REG));
 	} while ((reg & DSIM_PLL_STABLE) == 0);

 	return fout;
@@ -491,6 +636,8 @@  static int exynos_dsi_enable_clock(struct exynos_dsi *dsi)
 	unsigned long esc_div;
 	u32 reg;

+	reg = readl(REG(dsi, DSIM_STATUS_REG));
+
 	hs_clk = exynos_dsi_set_pll(dsi, dsi->burst_clk_rate);
 	if (!hs_clk) {
 		dev_err(dsi->dev, "failed to configure DSI PLL\n");
@@ -509,7 +656,7 @@  static int exynos_dsi_enable_clock(struct exynos_dsi *dsi)
 	dev_dbg(dsi->dev, "hs_clk = %lu, byte_clk = %lu, esc_clk = %lu\n",
 		hs_clk, byte_clk, esc_clk);

-	reg = readl(dsi->reg_base + DSIM_CLKCTRL_REG);
+	reg = readl(REG(dsi, DSIM_CLKCTRL_REG));
 	reg &= ~(DSIM_ESC_PRESCALER_MASK | DSIM_LANE_ESC_CLK_EN_CLK
 			| DSIM_LANE_ESC_CLK_EN_DATA_MASK | DSIM_PLL_BYPASS
 			| DSIM_BYTE_CLK_SRC_MASK);
@@ -519,7 +666,8 @@  static int exynos_dsi_enable_clock(struct exynos_dsi *dsi)
 			| DSIM_LANE_ESC_CLK_EN_DATA(BIT(dsi->lanes) - 1)
 			| DSIM_BYTE_CLK_SRC(0)
 			| DSIM_TX_REQUEST_HSCLK;
-	writel(reg, dsi->reg_base + DSIM_CLKCTRL_REG);
+
+	writel(reg, REG(dsi, DSIM_CLKCTRL_REG));

 	return 0;
 }
@@ -527,22 +675,24 @@  static int exynos_dsi_enable_clock(struct exynos_dsi *dsi)
 static void exynos_dsi_set_phy_ctrl(struct exynos_dsi *dsi)
 {
 	struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
+	unsigned int *values = driver_data->values;
 	u32 reg;

 	if (driver_data->has_freqband)
 		return;

 	/* B D-PHY: D-PHY Master & Slave Analog Block control */
-	reg = DSIM_PHYCTRL_ULPS_EXIT(0x0af);
-	writel(reg, dsi->reg_base + DSIM_PHYCTRL_REG);
+	reg = values[PHYCTRL_ULPS_EXIT] | values[PHYCTRL_VREG_LP] |
+		values[PHYCTRL_SLEW_UP];
+	writel(reg, REG(dsi, DSIM_PHYCTRL_REG));

 	/*
 	 * T LPX: Transmitted length of any Low-Power state period
 	 * T HS-EXIT: Time that the transmitter drives LP-11 following a HS
 	 *	burst
 	 */
-	reg = DSIM_PHYTIMING_LPX(0x06) | DSIM_PHYTIMING_HS_EXIT(0x0b);
-	writel(reg, dsi->reg_base + DSIM_PHYTIMING_REG);
+	reg = values[PHYTIMING_LPX] | values[PHYTIMING_HS_EXIT];
+	writel(reg, REG(dsi, DSIM_PHYTIMING_REG));

 	/*
 	 * T CLK-PREPARE: Time that the transmitter drives the Clock Lane LP-00
@@ -557,11 +707,10 @@  static void exynos_dsi_set_phy_ctrl(struct exynos_dsi *dsi)
 	 * T CLK-TRAIL: Time that the transmitter drives the HS-0 state after
 	 *	the last payload clock bit of a HS transmission burst
 	 */
-	reg = DSIM_PHYTIMING1_CLK_PREPARE(0x07) |
-			DSIM_PHYTIMING1_CLK_ZERO(0x27) |
-			DSIM_PHYTIMING1_CLK_POST(0x0d) |
-			DSIM_PHYTIMING1_CLK_TRAIL(0x08);
-	writel(reg, dsi->reg_base + DSIM_PHYTIMING1_REG);
+	reg = values[PHYTIMING_CLK_PREPARE] | values[PHYTIMING_CLK_ZERO] |
+		values[PHYTIMING_CLK_POST] | values[PHYTIMING_CLK_TRAIL];
+
+	writel(reg, REG(dsi, DSIM_PHYTIMING1_REG));

 	/*
 	 * T HS-PREPARE: Time that the transmitter drives the Data Lane LP-00
@@ -572,23 +721,23 @@  static void exynos_dsi_set_phy_ctrl(struct exynos_dsi *dsi)
 	 * T HS-TRAIL: Time that the transmitter drives the flipped differential
 	 *	state after last payload data bit of a HS transmission burst
 	 */
-	reg = DSIM_PHYTIMING2_HS_PREPARE(0x09) | DSIM_PHYTIMING2_HS_ZERO(0x0d) |
-			DSIM_PHYTIMING2_HS_TRAIL(0x0b);
-	writel(reg, dsi->reg_base + DSIM_PHYTIMING2_REG);
+	reg = values[PHYTIMING_HS_PREPARE] | values[PHYTIMING_HS_ZERO] |
+		values[PHYTIMING_HS_TRAIL];
+	writel(reg, REG(dsi, DSIM_PHYTIMING2_REG));
 }

 static void exynos_dsi_disable_clock(struct exynos_dsi *dsi)
 {
 	u32 reg;

-	reg = readl(dsi->reg_base + DSIM_CLKCTRL_REG);
+	reg = readl(REG(dsi, DSIM_CLKCTRL_REG));
 	reg &= ~(DSIM_LANE_ESC_CLK_EN_CLK | DSIM_LANE_ESC_CLK_EN_DATA_MASK
 			| DSIM_ESC_CLKEN | DSIM_BYTE_CLKEN);
-	writel(reg, dsi->reg_base + DSIM_CLKCTRL_REG);
+	writel(reg, REG(dsi, DSIM_CLKCTRL_REG));

-	reg = readl(dsi->reg_base + DSIM_PLLCTRL_REG);
+	reg = readl(REG(dsi, DSIM_PLLCTRL_REG));
 	reg &= ~DSIM_PLL_EN;
-	writel(reg, dsi->reg_base + DSIM_PLLCTRL_REG);
+	writel(reg, REG(dsi, DSIM_PLLCTRL_REG));
 }

 static int exynos_dsi_init_link(struct exynos_dsi *dsi)
@@ -599,15 +748,14 @@  static int exynos_dsi_init_link(struct exynos_dsi *dsi)
 	u32 lanes_mask;

 	/* Initialize FIFO pointers */
-	reg = readl(dsi->reg_base + DSIM_FIFOCTRL_REG);
+	reg = readl(REG(dsi, DSIM_FIFOCTRL_REG));
 	reg &= ~0x1f;
-	writel(reg, dsi->reg_base + DSIM_FIFOCTRL_REG);
+	writel(reg, REG(dsi, DSIM_FIFOCTRL_REG));

 	usleep_range(9000, 11000);

 	reg |= 0x1f;
-	writel(reg, dsi->reg_base + DSIM_FIFOCTRL_REG);
-
+	writel(reg, REG(dsi, DSIM_FIFOCTRL_REG));
 	usleep_range(9000, 11000);

 	/* DSI configuration */
@@ -666,14 +814,15 @@  static int exynos_dsi_init_link(struct exynos_dsi *dsi)

 	reg |= DSIM_NUM_OF_DATA_LANE(dsi->lanes - 1);

-	writel(reg, dsi->reg_base + DSIM_CONFIG_REG);
+	writel(reg, REG(dsi, DSIM_CONFIG_REG));

 	reg |= DSIM_LANE_EN_CLK;
-	writel(reg, dsi->reg_base + DSIM_CONFIG_REG);
+	writel(reg, REG(dsi, DSIM_CONFIG_REG));

 	lanes_mask = BIT(dsi->lanes) - 1;
 	reg |= DSIM_LANE_EN(lanes_mask);
-	writel(reg, dsi->reg_base + DSIM_CONFIG_REG);
+
+	writel(reg, REG(dsi, DSIM_CONFIG_REG));

 	/*
 	 * Use non-continuous clock mode if the periparal wants and
@@ -686,7 +835,7 @@  static int exynos_dsi_init_link(struct exynos_dsi *dsi)
 	if (driver_data->has_clklane_stop &&
 			dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
 		reg |= DSIM_CLKLANE_STOP;
-		writel(reg, dsi->reg_base + DSIM_CONFIG_REG);
+		writel(reg, REG(dsi, DSIM_CONFIG_REG));
 	}

 	/* Check clock and data lane state are stop state */
@@ -697,19 +846,19 @@  static int exynos_dsi_init_link(struct exynos_dsi *dsi)
 			return -EFAULT;
 		}

-		reg = readl(dsi->reg_base + DSIM_STATUS_REG);
+		reg = readl(REG(dsi, DSIM_STATUS_REG));
 		if ((reg & DSIM_STOP_STATE_DAT(lanes_mask))
 		    != DSIM_STOP_STATE_DAT(lanes_mask))
 			continue;
 	} while (!(reg & (DSIM_STOP_STATE_CLK | DSIM_TX_READY_HS_CLK)));

-	reg = readl(dsi->reg_base + DSIM_ESCMODE_REG);
+	reg = readl(REG(dsi, DSIM_ESCMODE_REG));
 	reg &= ~DSIM_STOP_STATE_CNT_MASK;
-	reg |= DSIM_STOP_STATE_CNT(0xf);
-	writel(reg, dsi->reg_base + DSIM_ESCMODE_REG);
+	reg |= DSIM_STOP_STATE_CNT(driver_data->values[STOP_STATE_CNT]);
+	writel(reg, REG(dsi, DSIM_ESCMODE_REG));

 	reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0xffff);
-	writel(reg, dsi->reg_base + DSIM_TIMEOUT_REG);
+	writel(reg, REG(dsi, DSIM_TIMEOUT_REG));

 	return 0;
 }
@@ -717,25 +866,27 @@  static int exynos_dsi_init_link(struct exynos_dsi *dsi)
 static void exynos_dsi_set_display_mode(struct exynos_dsi *dsi)
 {
 	struct videomode *vm = &dsi->vm;
+	unsigned int num_bits_resol = dsi->driver_data->num_bits_resol;
 	u32 reg;

 	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
 		reg = DSIM_CMD_ALLOW(0xf)
 			| DSIM_STABLE_VFP(vm->vfront_porch)
 			| DSIM_MAIN_VBP(vm->vback_porch);
-		writel(reg, dsi->reg_base + DSIM_MVPORCH_REG);
+		writel(reg, REG(dsi, DSIM_MVPORCH_REG));

 		reg = DSIM_MAIN_HFP(vm->hfront_porch)
 			| DSIM_MAIN_HBP(vm->hback_porch);
-		writel(reg, dsi->reg_base + DSIM_MHPORCH_REG);
+		writel(reg, REG(dsi, DSIM_MHPORCH_REG));

 		reg = DSIM_MAIN_VSA(vm->vsync_len)
 			| DSIM_MAIN_HSA(vm->hsync_len);
-		writel(reg, dsi->reg_base + DSIM_MSYNC_REG);
+		writel(reg, REG(dsi, DSIM_MSYNC_REG));
 	}
+	reg =  DSIM_MAIN_HRESOL(vm->hactive, num_bits_resol) |
+		DSIM_MAIN_VRESOL(vm->vactive, num_bits_resol);

-	reg = DSIM_MAIN_HRESOL(vm->hactive) | DSIM_MAIN_VRESOL(vm->vactive);
-	writel(reg, dsi->reg_base + DSIM_MDRESOL_REG);
+	writel(reg, REG(dsi, DSIM_MDRESOL_REG));

 	dev_dbg(dsi->dev, "LCD size = %dx%d\n", vm->hactive, vm->vactive);
 }
@@ -744,12 +895,12 @@  static void exynos_dsi_set_display_enable(struct exynos_dsi *dsi, bool enable)
 {
 	u32 reg;

-	reg = readl(dsi->reg_base + DSIM_MDRESOL_REG);
+	reg = readl(REG(dsi, DSIM_MDRESOL_REG));
 	if (enable)
 		reg |= DSIM_MAIN_STAND_BY;
 	else
 		reg &= ~DSIM_MAIN_STAND_BY;
-	writel(reg, dsi->reg_base + DSIM_MDRESOL_REG);
+	writel(reg, REG(dsi, DSIM_MDRESOL_REG));
 }

 static int exynos_dsi_wait_for_hdr_fifo(struct exynos_dsi *dsi)
@@ -757,7 +908,7 @@  static int exynos_dsi_wait_for_hdr_fifo(struct exynos_dsi *dsi)
 	int timeout = 2000;

 	do {
-		u32 reg = readl(dsi->reg_base + DSIM_FIFOCTRL_REG);
+		u32 reg = readl(REG(dsi, DSIM_FIFOCTRL_REG));

 		if (!(reg & DSIM_SFR_HEADER_FULL))
 			return 0;
@@ -771,22 +922,22 @@  static int exynos_dsi_wait_for_hdr_fifo(struct exynos_dsi *dsi)

 static void exynos_dsi_set_cmd_lpm(struct exynos_dsi *dsi, bool lpm)
 {
-	u32 v = readl(dsi->reg_base + DSIM_ESCMODE_REG);
+	u32 v = readl(REG(dsi, DSIM_ESCMODE_REG));

 	if (lpm)
 		v |= DSIM_CMD_LPDT_LP;
 	else
 		v &= ~DSIM_CMD_LPDT_LP;

-	writel(v, dsi->reg_base + DSIM_ESCMODE_REG);
+	writel(v, REG(dsi, DSIM_ESCMODE_REG));
 }

 static void exynos_dsi_force_bta(struct exynos_dsi *dsi)
 {
-	u32 v = readl(dsi->reg_base + DSIM_ESCMODE_REG);
+	u32 v = readl(REG(dsi, DSIM_ESCMODE_REG));

 	v |= DSIM_FORCE_BTA;
-	writel(v, dsi->reg_base + DSIM_ESCMODE_REG);
+	writel(v, REG(dsi, DSIM_ESCMODE_REG));
 }

 static void exynos_dsi_send_to_fifo(struct exynos_dsi *dsi,
@@ -810,7 +961,7 @@  static void exynos_dsi_send_to_fifo(struct exynos_dsi *dsi,
 	while (length >= 4) {
 		reg = (payload[3] << 24) | (payload[2] << 16)
 					| (payload[1] << 8) | payload[0];
-		writel(reg, dsi->reg_base + DSIM_PAYLOAD_REG);
+		writel(reg, REG(dsi, DSIM_PAYLOAD_REG));
 		payload += 4;
 		length -= 4;
 	}
@@ -825,7 +976,7 @@  static void exynos_dsi_send_to_fifo(struct exynos_dsi *dsi,
 		/* Fall through */
 	case 1:
 		reg |= payload[0];
-		writel(reg, dsi->reg_base + DSIM_PAYLOAD_REG);
+		writel(reg, REG(dsi, DSIM_PAYLOAD_REG));
 		break;
 	case 0:
 		/* Do nothing */
@@ -848,7 +999,7 @@  static void exynos_dsi_send_to_fifo(struct exynos_dsi *dsi,
 		dsi->state ^= DSIM_STATE_CMD_LPM;
 	}

-	writel(reg, dsi->reg_base + DSIM_PKTHDR_REG);
+	writel(reg, REG(dsi, DSIM_PKTHDR_REG));

 	if (xfer->flags & MIPI_DSI_MSG_REQ_ACK)
 		exynos_dsi_force_bta(dsi);
@@ -864,7 +1015,7 @@  static void exynos_dsi_read_from_fifo(struct exynos_dsi *dsi,
 	u32 reg;

 	if (first) {
-		reg = readl(dsi->reg_base + DSIM_RXFIFO_REG);
+		reg = readl(REG(dsi, DSIM_RXFIFO_REG));

 		switch (reg & 0x3f) {
 		case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_2BYTE:
@@ -903,7 +1054,7 @@  static void exynos_dsi_read_from_fifo(struct exynos_dsi *dsi,

 	/* Receive payload */
 	while (length >= 4) {
-		reg = readl(dsi->reg_base + DSIM_RXFIFO_REG);
+		reg = readl(REG(dsi, DSIM_RXFIFO_REG));
 		payload[0] = (reg >>  0) & 0xff;
 		payload[1] = (reg >>  8) & 0xff;
 		payload[2] = (reg >> 16) & 0xff;
@@ -913,7 +1064,7 @@  static void exynos_dsi_read_from_fifo(struct exynos_dsi *dsi,
 	}

 	if (length) {
-		reg = readl(dsi->reg_base + DSIM_RXFIFO_REG);
+		reg = readl(REG(dsi, DSIM_RXFIFO_REG));
 		switch (length) {
 		case 3:
 			payload[2] = (reg >> 16) & 0xff;
@@ -932,7 +1083,7 @@  static void exynos_dsi_read_from_fifo(struct exynos_dsi *dsi,
 clear_fifo:
 	length = DSI_RX_FIFO_SIZE / 4;
 	do {
-		reg = readl(dsi->reg_base + DSIM_RXFIFO_REG);
+		reg = readl(REG(dsi, DSIM_RXFIFO_REG));
 		if (reg == DSI_RX_FIFO_EMPTY)
 			break;
 	} while (--length);
@@ -1088,23 +1239,27 @@  static irqreturn_t exynos_dsi_irq(int irq, void *dev_id)
 	struct exynos_dsi *dsi = dev_id;
 	u32 status;

-	status = readl(dsi->reg_base + DSIM_INTSRC_REG);
+	status = readl(REG(dsi, DSIM_INTSRC_REG));
 	if (!status) {
 		static unsigned long int j;
 		if (printk_timed_ratelimit(&j, 500))
 			dev_warn(dsi->dev, "spurious interrupt\n");
 		return IRQ_HANDLED;
 	}
-	writel(status, dsi->reg_base + DSIM_INTSRC_REG);
+	writel(status, REG(dsi, DSIM_INTSRC_REG));

 	if (status & DSIM_INT_SW_RST_RELEASE) {
-		u32 mask = ~(DSIM_INT_RX_DONE | DSIM_INT_SFR_FIFO_EMPTY);
-		writel(mask, dsi->reg_base + DSIM_INTMSK_REG);
+		u32 mask = ~(DSIM_INT_RX_DONE | DSIM_INT_SFR_FIFO_EMPTY |
+			DSIM_INT_SFR_HDR_FIFO_EMPTY | DSIM_INT_FRAME_DONE |
+			DSIM_INT_RX_ECC_ERR | DSIM_INT_SW_RST_RELEASE);
+
+		writel(mask, REG(dsi, DSIM_INTMSK_REG));
 		complete(&dsi->completed);
 		return IRQ_HANDLED;
 	}

-	if (!(status & (DSIM_INT_RX_DONE | DSIM_INT_SFR_FIFO_EMPTY)))
+	if (!(status & (DSIM_INT_RX_DONE | DSIM_INT_SFR_FIFO_EMPTY |
+			DSIM_INT_FRAME_DONE | DSIM_INT_PLL_STABLE)))
 		return IRQ_HANDLED;

 	if (exynos_dsi_transfer_finish(dsi))
@@ -1142,10 +1297,13 @@  static void exynos_dsi_disable_irq(struct exynos_dsi *dsi)

 static int exynos_dsi_init(struct exynos_dsi *dsi)
 {
+	struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
+
 	exynos_dsi_reset(dsi);
 	exynos_dsi_enable_irq(dsi);
 	exynos_dsi_enable_clock(dsi);
-	exynos_dsi_wait_for_reset(dsi);
+	if (driver_data->wait_for_reset)
+		exynos_dsi_wait_for_reset(dsi);
 	exynos_dsi_set_phy_ctrl(dsi);
 	exynos_dsi_init_link(dsi);

@@ -1294,7 +1452,8 @@  static const struct mipi_dsi_host_ops exynos_dsi_ops = {

 static int exynos_dsi_poweron(struct exynos_dsi *dsi)
 {
-	int ret;
+	struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
+	int ret, i;

 	ret = regulator_bulk_enable(ARRAY_SIZE(dsi->supplies), dsi->supplies);
 	if (ret < 0) {
@@ -1302,31 +1461,24 @@  static int exynos_dsi_poweron(struct exynos_dsi *dsi)
 		return ret;
 	}

-	ret = clk_prepare_enable(dsi->bus_clk);
-	if (ret < 0) {
-		dev_err(dsi->dev, "cannot enable bus clock %d\n", ret);
-		goto err_bus_clk;
-	}
-
-	ret = clk_prepare_enable(dsi->pll_clk);
-	if (ret < 0) {
-		dev_err(dsi->dev, "cannot enable pll clock %d\n", ret);
-		goto err_pll_clk;
+	for (i = 0; i < driver_data->num_clks; i++) {
+		ret = clk_prepare_enable(dsi->clks[i]);
+		if (ret < 0)
+			goto err_clk;
 	}

 	ret = phy_power_on(dsi->phy);
 	if (ret < 0) {
 		dev_err(dsi->dev, "cannot enable phy %d\n", ret);
-		goto err_phy;
+		goto err_clk;
 	}

 	return 0;

-err_phy:
-	clk_disable_unprepare(dsi->pll_clk);
-err_pll_clk:
-	clk_disable_unprepare(dsi->bus_clk);
-err_bus_clk:
+err_clk:
+	while (--i > -1)
+		clk_disable_unprepare(dsi->clks[i]);
+
 	regulator_bulk_disable(ARRAY_SIZE(dsi->supplies), dsi->supplies);

 	return ret;
@@ -1334,7 +1486,8 @@  err_bus_clk:

 static void exynos_dsi_poweroff(struct exynos_dsi *dsi)
 {
-	int ret;
+	struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
+	int ret, i;

 	usleep_range(10000, 20000);

@@ -1350,8 +1503,8 @@  static void exynos_dsi_poweroff(struct exynos_dsi *dsi)

 	phy_power_off(dsi->phy);

-	clk_disable_unprepare(dsi->pll_clk);
-	clk_disable_unprepare(dsi->bus_clk);
+	for (i = driver_data->num_clks - 1; i > -1; i--)
+		clk_disable_unprepare(dsi->clks[i]);

 	ret = regulator_bulk_disable(ARRAY_SIZE(dsi->supplies), dsi->supplies);
 	if (ret < 0)
@@ -1680,7 +1833,7 @@  static int exynos_dsi_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	struct exynos_dsi *dsi;
-	int ret;
+	int ret, i;

 	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
 	if (!dsi)
@@ -1720,18 +1873,16 @@  static int exynos_dsi_probe(struct platform_device *pdev)
 		return -EPROBE_DEFER;
 	}

-	dsi->pll_clk = devm_clk_get(dev, "pll_clk");
-	if (IS_ERR(dsi->pll_clk)) {
-		dev_info(dev, "failed to get dsi pll input clock\n");
-		ret = PTR_ERR(dsi->pll_clk);
-		goto err_del_component;
-	}
-
-	dsi->bus_clk = devm_clk_get(dev, "bus_clk");
-	if (IS_ERR(dsi->bus_clk)) {
-		dev_info(dev, "failed to get dsi bus clock\n");
-		ret = PTR_ERR(dsi->bus_clk);
-		goto err_del_component;
+	dsi->clks = devm_kzalloc(dev,
+				sizeof(*dsi->clks) * dsi->driver_data->num_clks,
+				GFP_KERNEL);
+	for (i = 0; i < dsi->driver_data->num_clks; i++) {
+		dsi->clks[i] = devm_clk_get(dev, clk_names[i]);
+		if (IS_ERR(dsi->clks[i])) {
+			dev_info(dev, "failed to get dsi pll input clock\n");
+			ret = PTR_ERR(dsi->clks[i]);
+			goto err_del_component;
+		}
 	}

 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);