[PATCH 3/5] S5PC110: add machine specific MIPI-DSI setup code.

[PATCH 3/5] S5PC110: add machine specific MIPI-DSI setup code.

[Inki Dae]

Signed-off-by: Inki Dae <inki.dae@samsung.com>
---
 arch/arm/mach-s5pv210/Kconfig      |    6 +++
 arch/arm/mach-s5pv210/Makefile     |    1 +
 arch/arm/mach-s5pv210/setup-mipi.c |   76 ++++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-s5pv210/setup-mipi.c

diff --git a/arch/arm/mach-s5pv210/Kconfig b/arch/arm/mach-s5pv210/Kconfig
index 862f239..76da541 100644
--- a/arch/arm/mach-s5pv210/Kconfig
+++ b/arch/arm/mach-s5pv210/Kconfig
@@ -53,6 +53,11 @@ config S5PV210_SETUP_SDHCI_GPIO
 	help
 	  Common setup code for SDHCI gpio.
 
+config S5P_SETUP_MIPI_DSI
+	bool
+	help
+	  Common setup code for MIPI-DSI
+
 menu "S5PC110 Machines"
 
 config MACH_AQUILA
@@ -92,6 +97,7 @@ config MACH_GONI
 	select S5PV210_SETUP_I2C2
 	select S5PV210_SETUP_KEYPAD
 	select S5PV210_SETUP_SDHCI
+	select S5P_SETUP_MIPI_DSI
 	help
 	  Machine support for Samsung GONI board
 	  S5PC110(MCP) is one of package option of S5PV210
diff --git a/arch/arm/mach-s5pv210/Makefile b/arch/arm/mach-s5pv210/Makefile
index ff1a0db..638747c 100644
--- a/arch/arm/mach-s5pv210/Makefile
+++ b/arch/arm/mach-s5pv210/Makefile
@@ -37,3 +37,4 @@ obj-$(CONFIG_S5PV210_SETUP_IDE)		+= setup-ide.o
 obj-$(CONFIG_S5PV210_SETUP_KEYPAD)	+= setup-keypad.o
 obj-$(CONFIG_S5PV210_SETUP_SDHCI)       += setup-sdhci.o
 obj-$(CONFIG_S5PV210_SETUP_SDHCI_GPIO)	+= setup-sdhci-gpio.o
+obj-$(CONFIG_S5P_SETUP_MIPI_DSI)	+= setup-mipi.o
\ No newline at end of file
diff --git a/arch/arm/mach-s5pv210/setup-mipi.c b/arch/arm/mach-s5pv210/setup-mipi.c
new file mode 100644
index 0000000..2cc8cd1
--- /dev/null
+++ b/arch/arm/mach-s5pv210/setup-mipi.c
@@ -0,0 +1,76 @@
+/* linux/arch/arm/plat-s5p/setup-mipi.c
+ *
+ * Samsung MIPI-DSI DPHY driver.
+ *
+ * Author: InKi Dae <inki.dae@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+
+#include <mach/map.h>
+#include <mach/regs-clock.h>
+
+#include <plat/mipi-dsi.h>
+#include <plat/regs-dsim.h>
+
+static int s5p_mipi_enable_d_phy(struct dsim_device *dsim, unsigned int enable)
+{
+	unsigned int reg;
+
+	reg = readl(S5P_MIPI_CONTROL) & ~(1 << 0);
+	reg |= (enable << 0);
+	writel(reg, S5P_MIPI_CONTROL);
+
+	return 0;
+}
+
+static int s5p_mipi_enable_dsi_master(struct dsim_device *dsim,
+	unsigned int enable)
+{
+	unsigned int reg;
+
+	reg = readl(S5P_MIPI_CONTROL) & ~(1 << 2);
+	reg |= (enable << 2);
+	writel(reg, S5P_MIPI_CONTROL);
+
+	return 0;
+}
+
+int s5p_mipi_part_reset(struct dsim_device *dsim)
+{
+	writel(S5P_MIPI_M_RESETN, S5P_MIPI_PHY_CON0);
+
+	return 0;
+}
+
+int s5p_mipi_init_d_phy(struct dsim_device *dsim)
+{
+	/**
+	 * DPHY and Master block must be enabled@the system initialization
+	 * step before data access from/to DPHY begins.
+	 */
+	s5p_mipi_enable_d_phy(dsim, 1);
+
+	s5p_mipi_enable_dsi_master(dsim, 1);
+
+	return 0;
+}
-- 
1.7.0.4

[PATCH 3/5] S5PC110: add machine specific MIPI-DSI setup code.

[Kukjin Kim]

InKi Dae wrote:
> 
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> ---
>  arch/arm/mach-s5pv210/Kconfig      |    6 +++
>  arch/arm/mach-s5pv210/Makefile     |    1 +
>  arch/arm/mach-s5pv210/setup-mipi.c |   76
> ++++++++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-s5pv210/setup-mipi.c
> 
> diff --git a/arch/arm/mach-s5pv210/Kconfig b/arch/arm/mach-s5pv210/Kconfig
> index 862f239..76da541 100644
> --- a/arch/arm/mach-s5pv210/Kconfig
> +++ b/arch/arm/mach-s5pv210/Kconfig
> @@ -53,6 +53,11 @@ config S5PV210_SETUP_SDHCI_GPIO
>  	help
>  	  Common setup code for SDHCI gpio.
> 
> +config S5P_SETUP_MIPI_DSI

If this is for S5PV210, please use S5PV210_xxx as prefix. Or this is for S5P
SoCS, move into plat-s5p.

And as I know, this is _not_ only for MIPI DSI master...so need to re-name.

> +	bool
> +	help
> +	  Common setup code for MIPI-DSI
> +
>  menu "S5PC110 Machines"
> 
>  config MACH_AQUILA
> @@ -92,6 +97,7 @@ config MACH_GONI
>  	select S5PV210_SETUP_I2C2
>  	select S5PV210_SETUP_KEYPAD
>  	select S5PV210_SETUP_SDHCI
> +	select S5P_SETUP_MIPI_DSI

Is this really only for machine?

>  	help
>  	  Machine support for Samsung GONI board
>  	  S5PC110(MCP) is one of package option of S5PV210
> diff --git a/arch/arm/mach-s5pv210/Makefile
b/arch/arm/mach-s5pv210/Makefile
> index ff1a0db..638747c 100644
> --- a/arch/arm/mach-s5pv210/Makefile
> +++ b/arch/arm/mach-s5pv210/Makefile
> @@ -37,3 +37,4 @@ obj-$(CONFIG_S5PV210_SETUP_IDE)		+=
setup-ide.o
>  obj-$(CONFIG_S5PV210_SETUP_KEYPAD)	+= setup-keypad.o
>  obj-$(CONFIG_S5PV210_SETUP_SDHCI)       += setup-sdhci.o
>  obj-$(CONFIG_S5PV210_SETUP_SDHCI_GPIO)	+= setup-sdhci-gpio.o
> +obj-$(CONFIG_S5P_SETUP_MIPI_DSI)	+= setup-mipi.o
> \ No newline at end of file
> diff --git a/arch/arm/mach-s5pv210/setup-mipi.c b/arch/arm/mach-
> s5pv210/setup-mipi.c
> new file mode 100644
> index 0000000..2cc8cd1
> --- /dev/null
> +++ b/arch/arm/mach-s5pv210/setup-mipi.c
> @@ -0,0 +1,76 @@
> +/* linux/arch/arm/plat-s5p/setup-mipi.c
> + *
> + * Samsung MIPI-DSI DPHY driver.
> + *
> + * Author: InKi Dae <inki.dae@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +
> +#include <mach/map.h>
> +#include <mach/regs-clock.h>
> +
> +#include <plat/mipi-dsi.h>

Hmm...I didn't find this header in your previous patch.

> +#include <plat/regs-dsim.h>

Same.

> +
> +static int s5p_mipi_enable_d_phy(struct dsim_device *dsim, unsigned int
> enable)

Why need struct dsim_device in argument?
As I said, this is for MIPI DSI and MIPI CSI...right?

> +{
> +	unsigned int reg;
> +
> +	reg = readl(S5P_MIPI_CONTROL) & ~(1 << 0);

Please use __raw_readl here, because no need memory barrier between
operations.

> +	reg |= (enable << 0);
> +	writel(reg, S5P_MIPI_CONTROL);

Same.

> +
> +	return 0;

Always, return 0?

If enabled by MIPI DSI and MIPI CSI, how each IP can know other IP's MIPI
enalbling?

> +}
> +
> +static int s5p_mipi_enable_dsi_master(struct dsim_device *dsim,

Same.

> +	unsigned int enable)
> +{
> +	unsigned int reg;
> +
> +	reg = readl(S5P_MIPI_CONTROL) & ~(1 << 2);

Same.

> +	reg |= (enable << 2);
> +	writel(reg, S5P_MIPI_CONTROL);

Same.

> +
> +	return 0;
> +}
> +
> +int s5p_mipi_part_reset(struct dsim_device *dsim)

Do we really need argument, struct dsim_device?

> +{
> +	writel(S5P_MIPI_M_RESETN, S5P_MIPI_PHY_CON0);
> +
> +	return 0;
> +}
> +
> +int s5p_mipi_init_d_phy(struct dsim_device *dsim)
> +{
> +	/**
> +	 * DPHY and Master block must be enabled at the system
initialization
> +	 * step before data access from/to DPHY begins.
> +	 */
> +	s5p_mipi_enable_d_phy(dsim, 1);
> +
> +	s5p_mipi_enable_dsi_master(dsim, 1);
> +
> +	return 0;
> +}
> --

I can't get these needs...I mean need to re-think this for support MIPI
DSI(M) and MIPI CSI(S).
Actually MIPI control is needed in both.


Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 3/5] S5PC110: add machine specific MIPI-DSI setup code.

[daeinki]

Kukjin Kim ? ?:
> InKi Dae wrote:
>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>> ---
>>  arch/arm/mach-s5pv210/Kconfig      |    6 +++
>>  arch/arm/mach-s5pv210/Makefile     |    1 +
>>  arch/arm/mach-s5pv210/setup-mipi.c |   76
>> ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 83 insertions(+), 0 deletions(-)
>>  create mode 100644 arch/arm/mach-s5pv210/setup-mipi.c
>>
>> diff --git a/arch/arm/mach-s5pv210/Kconfig b/arch/arm/mach-s5pv210/Kconfig
>> index 862f239..76da541 100644
>> --- a/arch/arm/mach-s5pv210/Kconfig
>> +++ b/arch/arm/mach-s5pv210/Kconfig
>> @@ -53,6 +53,11 @@ config S5PV210_SETUP_SDHCI_GPIO
>>  	help
>>  	  Common setup code for SDHCI gpio.
>>
>> +config S5P_SETUP_MIPI_DSI
> 
> If this is for S5PV210, please use S5PV210_xxx as prefix. Or this is for S5P
> SoCS, move into plat-s5p.
> 
> And as I know, this is _not_ only for MIPI DSI master...so need to re-name.
> 
Ok, moved to plat-s5p.

>> +	bool
>> +	help
>> +	  Common setup code for MIPI-DSI
>> +
>>  menu "S5PC110 Machines"
>>
>>  config MACH_AQUILA
>> @@ -92,6 +97,7 @@ config MACH_GONI
>>  	select S5PV210_SETUP_I2C2
>>  	select S5PV210_SETUP_KEYPAD
>>  	select S5PV210_SETUP_SDHCI
>> +	select S5P_SETUP_MIPI_DSI
> 
> Is this really only for machine?
> 
>>  	help
>>  	  Machine support for Samsung GONI board
>>  	  S5PC110(MCP) is one of package option of S5PV210
>> diff --git a/arch/arm/mach-s5pv210/Makefile
> b/arch/arm/mach-s5pv210/Makefile
>> index ff1a0db..638747c 100644
>> --- a/arch/arm/mach-s5pv210/Makefile
>> +++ b/arch/arm/mach-s5pv210/Makefile
>> @@ -37,3 +37,4 @@ obj-$(CONFIG_S5PV210_SETUP_IDE)		+=
> setup-ide.o
>>  obj-$(CONFIG_S5PV210_SETUP_KEYPAD)	+= setup-keypad.o
>>  obj-$(CONFIG_S5PV210_SETUP_SDHCI)       += setup-sdhci.o
>>  obj-$(CONFIG_S5PV210_SETUP_SDHCI_GPIO)	+= setup-sdhci-gpio.o
>> +obj-$(CONFIG_S5P_SETUP_MIPI_DSI)	+= setup-mipi.o
>> \ No newline at end of file
>> diff --git a/arch/arm/mach-s5pv210/setup-mipi.c b/arch/arm/mach-
>> s5pv210/setup-mipi.c
>> new file mode 100644
>> index 0000000..2cc8cd1
>> --- /dev/null
>> +++ b/arch/arm/mach-s5pv210/setup-mipi.c
>> @@ -0,0 +1,76 @@
>> +/* linux/arch/arm/plat-s5p/setup-mipi.c
>> + *
>> + * Samsung MIPI-DSI DPHY driver.
>> + *
>> + * Author: InKi Dae <inki.dae@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +#include <linux/kernel.h>
>> +#include <linux/string.h>
>> +#include <linux/io.h>
>> +#include <linux/err.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.h>
>> +
>> +#include <mach/map.h>
>> +#include <mach/regs-clock.h>
>> +
>> +#include <plat/mipi-dsi.h>
> 
> Hmm...I didn't find this header in your previous patch.
>
previous patch??

>> +#include <plat/regs-dsim.h>
> 
> Same.
> 
>> +
>> +static int s5p_mipi_enable_d_phy(struct dsim_device *dsim, unsigned int
>> enable)
> 
> Why need struct dsim_device in argument?
> As I said, this is for MIPI DSI and MIPI CSI...right?
> 
>> +{
>> +	unsigned int reg;
>> +
>> +	reg = readl(S5P_MIPI_CONTROL) & ~(1 << 0);
> 
> Please use __raw_readl here, because no need memory barrier between
> operations.
> 
>> +	reg |= (enable << 0);
>> +	writel(reg, S5P_MIPI_CONTROL);
> 
> Same.
> 
>> +
>> +	return 0;
> 
> Always, return 0?
> 
> If enabled by MIPI DSI and MIPI CSI, how each IP can know other IP's MIPI
> enalbling?
> 
ok, naming issue sould be considered more.
hm, how about using "DSIM"? I think S5P_MIPI_DSI or MIPI_DSI is too long.
>> +}
>> +
>> +static int s5p_mipi_enable_dsi_master(struct dsim_device *dsim,
> 
> Same.
> 
>> +	unsigned int enable)
>> +{
>> +	unsigned int reg;
>> +
>> +	reg = readl(S5P_MIPI_CONTROL) & ~(1 << 2);
> 
> Same.
> 
>> +	reg |= (enable << 2);
>> +	writel(reg, S5P_MIPI_CONTROL);
> 
> Same.
> 
>> +
>> +	return 0;
>> +}
>> +
>> +int s5p_mipi_part_reset(struct dsim_device *dsim)
> 
> Do we really need argument, struct dsim_device?
> 
It doesn't. setup-mipi.c is specific to SoC platform.
these functions should be registered to mipi-dsi platform data as 
callbacks in other words, mipi-dsi driver calls them so according to 
mipi-dsi master framework rule, I added dsim_device as argument.
anyway, don't care.
>> +{
>> +	writel(S5P_MIPI_M_RESETN, S5P_MIPI_PHY_CON0);
>> +
>> +	return 0;
>> +}
>> +
>> +int s5p_mipi_init_d_phy(struct dsim_device *dsim)
>> +{
>> +	/**
>> +	 * DPHY and Master block must be enabled at the system
> initialization
>> +	 * step before data access from/to DPHY begins.
>> +	 */
>> +	s5p_mipi_enable_d_phy(dsim, 1);
>> +
>> +	s5p_mipi_enable_dsi_master(dsim, 1);
>> +
>> +	return 0;
>> +}
>> --
> 
> I can't get these needs...I mean need to re-think this for support MIPI
> DSI(M) and MIPI CSI(S).
> Actually MIPI control is needed in both.
> 
I agree to your opinion.
it would be corrected, "MIPI" -> "DSIM"
> 
> Thanks.
> 
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
> 
>