| Message ID | 1414656270-8048-2-git-send-email-pankaj.dubey@samsung.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Thu, Oct 30, 2014 at 01:34:29PM +0530, Pankaj Dubey wrote: > Let's handle i2c interrupt re-configuration in i2c driver. This will > help us in removing some soc specific checks from machine files and > will help in removing static iomapping of SYS register in exynos.c > > Since only Exynos5250, and Exynos5420 has i2c nodes in DT, added syscon > based phandle to i2c device nodes of respective SoC DT files. > > Also handle saving and restoring of SYS_I2C_CFG register during > suspend and resume of i2c driver. > > CC: Rob Herring <robh+dt@kernel.org> > CC: Randy Dunlap <rdunlap@infradead.org> > CC: Wolfram Sang <wsa@the-dreams.de> > CC: Russell King <linux@arm.linux.org.uk> > CC: devicetree@vger.kernel.org > CC: linux-doc@vger.kernel.org > CC: linux-i2c@vger.kernel.org > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > --- > .../devicetree/bindings/i2c/i2c-s3c2410.txt | 1 + > arch/arm/boot/dts/exynos5250.dtsi | 4 +++ > arch/arm/boot/dts/exynos5420.dtsi | 4 +++ I usually don't take DTS patches. They should go via arm-soc. Please say so if there are reasons I should take them. > @@ -1084,6 +1092,23 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) > of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr); > of_property_read_u32(np, "samsung,i2c-max-bus-freq", > (u32 *)&pdata->frequency); > + /* > + * Exynos5's legacy i2c controller and new high speed i2c > + * controller have muxed interrupt sources. By default the > + * interrupts for 4-channel HS-I2C controller are enabled. > + * If node for first four channels of legacy i2c controller s/node/nodes/ > + * are available then re-configure the interrupts via the > + * system register. > + */ > + id = of_alias_get_id(np, "i2c"); > + i2c->sysreg = syscon_regmap_lookup_by_phandle(np, > + "samsung,sysreg-phandle"); > + if (IS_ERR(i2c->sysreg)) { > + /* As this is not compulsory do not return error */ > + pr_info("i2c-%d skipping re-configuration of interrutps\n", id); I'd say drop this message. If you want to keep it, it should be dev_dbg. > + return; > + } > + regmap_update_bits(i2c->sysreg, EXYNOS5_SYS_I2C_CFG, BIT(id), 0); > } Rest looks good, thanks!
On Friday 21 November 2014 12:55 PM, Wolfram Sang wrote: > On Thu, Oct 30, 2014 at 01:34:29PM +0530, Pankaj Dubey wrote: >> Let's handle i2c interrupt re-configuration in i2c driver. This will >> help us in removing some soc specific checks from machine files and >> will help in removing static iomapping of SYS register in exynos.c >> >> Since only Exynos5250, and Exynos5420 has i2c nodes in DT, added syscon >> based phandle to i2c device nodes of respective SoC DT files. >> >> Also handle saving and restoring of SYS_I2C_CFG register during >> suspend and resume of i2c driver. >> >> CC: Rob Herring <robh+dt@kernel.org> >> CC: Randy Dunlap <rdunlap@infradead.org> >> CC: Wolfram Sang <wsa@the-dreams.de> >> CC: Russell King <linux@arm.linux.org.uk> >> CC: devicetree@vger.kernel.org >> CC: linux-doc@vger.kernel.org >> CC: linux-i2c@vger.kernel.org >> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> >> --- >> .../devicetree/bindings/i2c/i2c-s3c2410.txt | 1 + >> arch/arm/boot/dts/exynos5250.dtsi | 4 +++ >> arch/arm/boot/dts/exynos5420.dtsi | 4 +++ > > I usually don't take DTS patches. They should go via arm-soc. Please say > so if there are reasons I should take them. I CC'ed to you because same patch contains changes in i2c driver. I am not very sure via which tree this should go. May be I can ask samsung SoC maintainer Kukjin to look into this, as patch 2/2 has changes in mach-exynos which should go via Kukjin's tree. > >> @@ -1084,6 +1092,23 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) >> of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr); >> of_property_read_u32(np, "samsung,i2c-max-bus-freq", >> (u32 *)&pdata->frequency); >> + /* >> + * Exynos5's legacy i2c controller and new high speed i2c >> + * controller have muxed interrupt sources. By default the >> + * interrupts for 4-channel HS-I2C controller are enabled. >> + * If node for first four channels of legacy i2c controller > > s/node/nodes/ OK. > >> + * are available then re-configure the interrupts via the >> + * system register. >> + */ >> + id = of_alias_get_id(np, "i2c"); >> + i2c->sysreg = syscon_regmap_lookup_by_phandle(np, >> + "samsung,sysreg-phandle"); >> + if (IS_ERR(i2c->sysreg)) { >> + /* As this is not compulsory do not return error */ >> + pr_info("i2c-%d skipping re-configuration of interrutps\n", id); > > I'd say drop this message. If you want to keep it, it should be dev_dbg. OK. > >> + return; >> + } >> + regmap_update_bits(i2c->sysreg, EXYNOS5_SYS_I2C_CFG, BIT(id), 0); >> } > > Rest looks good, thanks! Thanks for review. Pankaj Dubey > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >I usually don't take DTS patches. They should go via arm-soc. Please say > >so if there are reasons I should take them. > > I CC'ed to you because same patch contains changes in i2c driver. Yes, those should absolutely go via my I2C tree. You need to make a seperate patch out of the dts changes which then also should go via samsung-soc, unless Kukjin says he really wants to go the via I2C. But I guess the latter will just create merge conflicts.
Pankaj Dubey wrote: > > On Friday 21 November 2014 12:55 PM, Wolfram Sang wrote: > > On Thu, Oct 30, 2014 at 01:34:29PM +0530, Pankaj Dubey wrote: > >> Let's handle i2c interrupt re-configuration in i2c driver. This will > >> help us in removing some soc specific checks from machine files and > >> will help in removing static iomapping of SYS register in exynos.c > >> > >> Since only Exynos5250, and Exynos5420 has i2c nodes in DT, added syscon > >> based phandle to i2c device nodes of respective SoC DT files. > >> Well...actually there are 4ch i2c in exynos5410 and exynos5800 as well, i2c nodes are not added in dt files though. > >> Also handle saving and restoring of SYS_I2C_CFG register during > >> suspend and resume of i2c driver. > >> > >> CC: Rob Herring <robh+dt@kernel.org> > >> CC: Randy Dunlap <rdunlap@infradead.org> > >> CC: Wolfram Sang <wsa@the-dreams.de> > >> CC: Russell King <linux@arm.linux.org.uk> > >> CC: devicetree@vger.kernel.org > >> CC: linux-doc@vger.kernel.org > >> CC: linux-i2c@vger.kernel.org > >> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > >> --- > >> .../devicetree/bindings/i2c/i2c-s3c2410.txt | 1 + > >> arch/arm/boot/dts/exynos5250.dtsi | 4 +++ > >> arch/arm/boot/dts/exynos5420.dtsi | 4 +++ > > > > I usually don't take DTS patches. They should go via arm-soc. Please say > > so if there are reasons I should take them. > > I CC'ed to you because same patch contains changes in i2c driver. > I am not very sure via which tree this should go. May be I can ask > samsung SoC maintainer Kukjin to look into this, as patch 2/2 has > changes in mach-exynos which should go via Kukjin's tree. > I'll reply on other email. Thanks, Kukjin -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Wolfram Sang wrote: > Hi Wolfram, > > >I usually don't take DTS patches. They should go via arm-soc. Please say > > >so if there are reasons I should take them. > > > > I CC'ed to you because same patch contains changes in i2c driver. > > Yes, those should absolutely go via my I2C tree. You need to make a > seperate patch out of the dts changes which then also should go via > samsung-soc, unless Kukjin says he really wants to go the via I2C. But I > guess the latter will just create merge conflicts. Hmm...I think, Pankaj needs to submit separated patches 1) driver change, 2) dt change and then 3) remove change. And 2nd and 3rd changes should be handed in Samsung tree together after landing 1) change in -next. Of course, 1) change should be handled in i2c tree ;) Thanks, Kukjin -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday 22 November 2014 06:18 AM, Kukjin Kim wrote: > Wolfram Sang wrote: >> > Hi Wolfram, > >>>> I usually don't take DTS patches. They should go via arm-soc. Please say >>>> so if there are reasons I should take them. >>> >>> I CC'ed to you because same patch contains changes in i2c driver. >> >> Yes, those should absolutely go via my I2C tree. You need to make a >> seperate patch out of the dts changes which then also should go via >> samsung-soc, unless Kukjin says he really wants to go the via I2C. But I >> guess the latter will just create merge conflicts. > > Hmm...I think, Pankaj needs to submit separated patches 1) driver change, 2) dt > change and then 3) remove change. And 2nd and 3rd changes should be handed in > Samsung tree together after landing 1) change in -next. > > Of course, 1) change should be handled in i2c tree ;) > Thanks for review and guidance. I separated i2c driver changes and posted it as v6 here [1]. DT changes and mach-exynos removal of i2c settings have been posted as v6 here [2]. Please do review and if OK let's get it merged. [1]: i2c-driver: https://patchwork.kernel.org/patch/5363981/ [2]: mach-exynos: http://comments.gmane.org/gmane.linux.kernel.samsung-soc/40155 Thanks, Pankaj Dubey > Thanks, > Kukjin > > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/i2c/i2c-s3c2410.txt b/Documentation/devicetree/bindings/i2c/i2c-s3c2410.txt index 278de8e..89b3250 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-s3c2410.txt +++ b/Documentation/devicetree/bindings/i2c/i2c-s3c2410.txt @@ -32,6 +32,7 @@ Optional properties: specified, default value is 0. - samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not specified, the default value in Hz is 100000. + - samsung,sysreg-phandle - handle to syscon used to control the system registers Example: diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 012b021..6a32d50 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -293,6 +293,7 @@ clock-names = "i2c"; pinctrl-names = "default"; pinctrl-0 = <&i2c0_bus>; + samsung,sysreg-phandle = <&sysreg_system_controller>; status = "disabled"; }; @@ -306,6 +307,7 @@ clock-names = "i2c"; pinctrl-names = "default"; pinctrl-0 = <&i2c1_bus>; + samsung,sysreg-phandle = <&sysreg_system_controller>; status = "disabled"; }; @@ -319,6 +321,7 @@ clock-names = "i2c"; pinctrl-names = "default"; pinctrl-0 = <&i2c2_bus>; + samsung,sysreg-phandle = <&sysreg_system_controller>; status = "disabled"; }; @@ -332,6 +335,7 @@ clock-names = "i2c"; pinctrl-names = "default"; pinctrl-0 = <&i2c3_bus>; + samsung,sysreg-phandle = <&sysreg_system_controller>; status = "disabled"; }; diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi index 8617a03..90bf401 100644 --- a/arch/arm/boot/dts/exynos5420.dtsi +++ b/arch/arm/boot/dts/exynos5420.dtsi @@ -560,6 +560,7 @@ clock-names = "i2c"; pinctrl-names = "default"; pinctrl-0 = <&i2c0_bus>; + samsung,sysreg-phandle = <&sysreg_system_controller>; status = "disabled"; }; @@ -573,6 +574,7 @@ clock-names = "i2c"; pinctrl-names = "default"; pinctrl-0 = <&i2c1_bus>; + samsung,sysreg-phandle = <&sysreg_system_controller>; status = "disabled"; }; @@ -586,6 +588,7 @@ clock-names = "i2c"; pinctrl-names = "default"; pinctrl-0 = <&i2c2_bus>; + samsung,sysreg-phandle = <&sysreg_system_controller>; status = "disabled"; }; @@ -599,6 +602,7 @@ clock-names = "i2c"; pinctrl-names = "default"; pinctrl-0 = <&i2c3_bus>; + samsung,sysreg-phandle = <&sysreg_system_controller>; status = "disabled"; }; diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index e3b0337..a46435a 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -39,6 +39,8 @@ #include <linux/of.h> #include <linux/of_gpio.h> #include <linux/pinctrl/consumer.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> #include <asm/irq.h> @@ -91,6 +93,9 @@ /* Max time to wait for bus to become idle after a xfer (in us) */ #define S3C2410_IDLE_TIMEOUT 5000 +/* Exynos5 Sysreg offset */ +#define EXYNOS5_SYS_I2C_CFG 0x0234 + /* i2c controller state */ enum s3c24xx_i2c_state { STATE_IDLE, @@ -127,6 +132,8 @@ struct s3c24xx_i2c { #if defined(CONFIG_ARM_S3C24XX_CPUFREQ) struct notifier_block freq_transition; #endif + struct regmap *sysreg; + unsigned int sys_i2c_cfg; }; static struct platform_device_id s3c24xx_driver_ids[] = { @@ -1075,6 +1082,7 @@ static void s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) { struct s3c2410_platform_i2c *pdata = i2c->pdata; + int id; if (!np) return; @@ -1084,6 +1092,23 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr); of_property_read_u32(np, "samsung,i2c-max-bus-freq", (u32 *)&pdata->frequency); + /* + * Exynos5's legacy i2c controller and new high speed i2c + * controller have muxed interrupt sources. By default the + * interrupts for 4-channel HS-I2C controller are enabled. + * If node for first four channels of legacy i2c controller + * are available then re-configure the interrupts via the + * system register. + */ + id = of_alias_get_id(np, "i2c"); + i2c->sysreg = syscon_regmap_lookup_by_phandle(np, + "samsung,sysreg-phandle"); + if (IS_ERR(i2c->sysreg)) { + /* As this is not compulsory do not return error */ + pr_info("i2c-%d skipping re-configuration of interrutps\n", id); + return; + } + regmap_update_bits(i2c->sysreg, EXYNOS5_SYS_I2C_CFG, BIT(id), 0); } #else static void @@ -1264,6 +1289,9 @@ static int s3c24xx_i2c_suspend_noirq(struct device *dev) i2c->suspended = 1; + if (!IS_ERR(i2c->sysreg)) + regmap_read(i2c->sysreg, EXYNOS5_SYS_I2C_CFG, &i2c->sys_i2c_cfg); + return 0; } @@ -1272,6 +1300,9 @@ static int s3c24xx_i2c_resume_noirq(struct device *dev) struct platform_device *pdev = to_platform_device(dev); struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev); + if (!IS_ERR(i2c->sysreg)) + regmap_write(i2c->sysreg, EXYNOS5_SYS_I2C_CFG, i2c->sys_i2c_cfg); + clk_prepare_enable(i2c->clk); s3c24xx_i2c_init(i2c); clk_disable_unprepare(i2c->clk);
Let's handle i2c interrupt re-configuration in i2c driver. This will help us in removing some soc specific checks from machine files and will help in removing static iomapping of SYS register in exynos.c Since only Exynos5250, and Exynos5420 has i2c nodes in DT, added syscon based phandle to i2c device nodes of respective SoC DT files. Also handle saving and restoring of SYS_I2C_CFG register during suspend and resume of i2c driver. CC: Rob Herring <robh+dt@kernel.org> CC: Randy Dunlap <rdunlap@infradead.org> CC: Wolfram Sang <wsa@the-dreams.de> CC: Russell King <linux@arm.linux.org.uk> CC: devicetree@vger.kernel.org CC: linux-doc@vger.kernel.org CC: linux-i2c@vger.kernel.org Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> --- .../devicetree/bindings/i2c/i2c-s3c2410.txt | 1 + arch/arm/boot/dts/exynos5250.dtsi | 4 +++ arch/arm/boot/dts/exynos5420.dtsi | 4 +++ drivers/i2c/busses/i2c-s3c2410.c | 31 ++++++++++++++++++++ 4 files changed, 40 insertions(+)