| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <000301cf89e3$a43d3550$ecb79ff0$@samsung.com>
Date: Tue, 17 Jun 2014 09:51:39 +0530
From: Pankaj Dubey <pankaj.dubey@...sung.com>
To: 'Tomasz Figa' <t.figa@...sung.com>,
linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Cc: kgene.kim@...sung.com, arnd@...db.de,
'Rob Herring' <robh+dt@...nel.org>,
'Randy Dunlap' <rdunlap@...radead.org>,
'Wolfram Sang' <wsa@...-dreams.de>,
'Russell King' <linux@....linux.org.uk>,
devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
linux-i2c@...r.kernel.org
Subject: RE: [PATCH v3 1/6] i2c: s3c2410: Handle i2c sys_cfg register in i2c
driver
Hi Tomasz,
>
> Hi Pankaj,
>
> On 10.05.2014 09:20, 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.
> > Since only Exynos5250, and Exynos5420 need to do this, 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. This will help in removing soc
> > specific check from mach-exynos/pm.c.
> >
> > CC: Rob Herring <robh+dt@...nel.org>
> > CC: Randy Dunlap <rdunlap@...radead.org>
> > CC: Wolfram Sang <wsa@...-dreams.de>
> > CC: Russell King <linux@....linux.org.uk>
> > CC: devicetree@...r.kernel.org
> > CC: linux-doc@...r.kernel.org
> > CC: linux-i2c@...r.kernel.org
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@...sung.com>
> > ---
> > .../devicetree/bindings/arm/samsung/sysreg.txt | 1 +
> > arch/arm/boot/dts/exynos5.dtsi | 5 +++
> > arch/arm/boot/dts/exynos5250.dtsi | 4 +++
> > arch/arm/boot/dts/exynos5420.dtsi | 4 +++
>
> What about Exynos5410 and Exynos5800?
OK, I will check user manuals of these two SoCs and if required will update.
>
> > drivers/i2c/busses/i2c-s3c2410.c | 32
> ++++++++++++++++++++
> > 5 files changed, 46 insertions(+)
>
> [snip]
>
> > diff --git a/drivers/i2c/busses/i2c-s3c2410.c
> > b/drivers/i2c/busses/i2c-s3c2410.c
> > index ae44910..e707062 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_i2s_cfg;
>
> typo: s/i2s/i2c/
Oops, sorry I will correct this.
>
> > };
> >
> > static struct platform_device_id s3c24xx_driver_ids[] = { @@ -1075,6
> > +1082,8 @@ static void s3c24xx_i2c_parse_dt(struct device_node *np,
> > struct s3c24xx_i2c *i2c) {
> > struct s3c2410_platform_i2c *pdata = i2c->pdata;
> > + u32 val = 0;
> > + int id;
> >
> > if (!np)
> > return;
> > @@ -1084,6 +1093,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
>
> What do you mean by "by default"? Is it a setting from the bootloader or
reset value?
>
It's a reset value.
> Probably to ensure that the mux is set correctly, same thing should be
also added to
> hsi2c driver.
>
> > + * 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,syscon-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), val);
>
> As Wolfram pointed, val can be replaced with immediate 0 here.
OK, I will take care as per suggestion.
>
> > }
> > #else
> > static void
> > @@ -1268,6 +1294,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_i2s_cfg);
> > +
> > return 0;
> > }
> >
> > @@ -1276,6 +1305,9 @@ static int s3c24xx_i2c_resume(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, i2c->sys_i2s_cfg,
> EXYNOS5_SYS_I2C_CFG);
>
> According to include/linux/regmap.h:
>
> int regmap_write(struct regmap *map, unsigned int reg, unsigned int val);
>
> So in your code reg is swapped with val.
Yes. I will correct this.
>
> Best regards,
> Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists