Re: [PATCH v3] arm64: dts: add all hi6220 i2c nodes
On Wed, Dec 02, 2015 at 02:29:09PM +0800, Xinwei Kong wrote:
> This patch adds all I2C nodes for the Hi6220 SoC. This hi6220 Soc
> use this I2C IP of Synopsys Designware for HiKey board.
>
> Signed-off-by: Xinwei Kong
> ---
> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 31
> +++
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 82d2488..6eae673 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -208,5 +208,36 @@
> clock-names = "uartclk", "apb_pclk";
> status = "disabled";
> };
> +
> + i2c0: i2c@f710 {
> + compatible = "snps,designware-i2c";
> + reg = <0x0 0xf710 0x0 0x1000>;
> + interrupts = <0 44 4>;
> + i2c-sda-hold-time-ns = <300>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>;
> + status = "disabled";
You got me wrong. I was asking you to drop property 'clock-names' only,
not 'clocks' together. I do not think it works if you drop property
'clocks', because I see that dw_i2c_plat_probe() will fail if
devm_clk_get() fails. Did you test the patch before posting it out?
Shawn
> + };
> +
> + i2c1: i2c@f7101000 {
> + compatible = "snps,designware-i2c";
> + reg = <0x0 0xf7101000 0x0 0x1000>;
> + interrupts = <0 45 4>;
> + i2c-sda-hold-time-ns = <300>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pmx_func &i2c1_cfg_func>;
> + status = "disabled";
> + };
> +
> + i2c2: i2c@f7102000 {
> + compatible = "snps,designware-i2c";
> + reg = <0x0 0xf7102000 0x0 0x1000>;
> + interrupts = <0 46 4>;
> + i2c-sda-hold-time-ns = <300>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c2_pmx_func &i2c2_cfg_func>;
> + status = "disabled";
> + };
> +
> };
> };
> --
> 1.9.1
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] arm64: dts: add all hi6220 i2c nodes
This patch adds all I2C nodes for the Hi6220 SoC. This hi6220 Soc
use this I2C IP of Synopsys Designware for HiKey board.
Signed-off-by: Xinwei Kong
---
arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 31 +++
1 file changed, 31 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 82d2488..6eae673 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -208,5 +208,36 @@
clock-names = "uartclk", "apb_pclk";
status = "disabled";
};
+
+ i2c0: i2c@f710 {
+ compatible = "snps,designware-i2c";
+ reg = <0x0 0xf710 0x0 0x1000>;
+ interrupts = <0 44 4>;
+ i2c-sda-hold-time-ns = <300>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>;
+ status = "disabled";
+ };
+
+ i2c1: i2c@f7101000 {
+ compatible = "snps,designware-i2c";
+ reg = <0x0 0xf7101000 0x0 0x1000>;
+ interrupts = <0 45 4>;
+ i2c-sda-hold-time-ns = <300>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c1_pmx_func &i2c1_cfg_func>;
+ status = "disabled";
+ };
+
+ i2c2: i2c@f7102000 {
+ compatible = "snps,designware-i2c";
+ reg = <0x0 0xf7102000 0x0 0x1000>;
+ interrupts = <0 46 4>;
+ i2c-sda-hold-time-ns = <300>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_pmx_func &i2c2_cfg_func>;
+ status = "disabled";
+ };
+
};
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] i2c: mediatek: fix i2c multi transfer issue in high speed mode
On Sat, 2015-11-14 at 22:38 +0800, Daniel Kurtz wrote:
> On Mon, Nov 9, 2015 at 1:43 PM, Liguo Zhang wrote:
> > For platform with auto restart support, when doing i2c multi transfer
> > in high speed, for example, doing write-then-read transfer, the master
> > code will occupy the first transfer, and the second transfer will be
> > the read transfer, the write transfer will be discarded. So we should
> > first send the master code, and then start i2c multi transfer.
> >
> > Signed-off-by: Liguo Zhang
> > Reviewed-by: Eddie Huang
> > ---
> > drivers/i2c/busses/i2c-mt65xx.c | 45
> > +
> > 1 file changed, 45 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-mt65xx.c
> > b/drivers/i2c/busses/i2c-mt65xx.c
> > index dc4aac6..249df86 100644
> > --- a/drivers/i2c/busses/i2c-mt65xx.c
> > +++ b/drivers/i2c/busses/i2c-mt65xx.c
> > @@ -53,6 +53,8 @@
> > #define I2C_FS_TIME_INIT_VALUE 0x1303
> > #define I2C_WRRD_TRANAC_VALUE 0x0002
> > #define I2C_RD_TRANAC_VALUE0x0001
> > +#define I2C_TRAN_DEFAULT_VALUE 0x0001
> > +#define I2C_TRANAC_DEFAULT_VALUE 0x0001
>
> "TRAN" and "TRANAC" are not good names; this should be "TRANSFER_LEN"
> and "TRANSAC", based on the names of the registers to which you write
> these constants.
>
> Furthermore, these are not "default" values, they are the transfer
> length and number of transactions for sending the "master code", so:
>
> #define I2C_TRANSFER_LEN_MASTER_CODE 0x0001
> #define I2C_TRANSAC_LEN_MASTER_CODE 0x0001
>
> Similarly, I think the "TRANAC" in I2C_WRRD_TRANAC_VALUE and
> I2C_RD_TRANAC_VALUE should also be TRANSAC.
>
Yes, I will follow your advice.
> >
> > #define I2C_DMA_CON_TX 0x
> > #define I2C_DMA_CON_RX 0x0001
> > @@ -365,6 +367,43 @@ static int mtk_i2c_set_speed(struct mtk_i2c *i2c,
> > unsigned int parent_clk,
> > return 0;
> > }
> >
> > +static int mtk_i2c_send_master_code(struct mtk_i2c *i2c)
> > +{
> > + int ret = 0;
> > +
> > + reinit_completion(&i2c->msg_complete);
> > +
> > + writew(I2C_CONTROL_RS | I2C_CONTROL_ACKERR_DET_EN |
> > + I2C_CONTROL_CLK_EXT_EN | I2C_CONTROL_DMA_EN,
> > + i2c->base + OFFSET_CONTROL);
> > +
> > + /* Clear interrupt status */
> > + writew(I2C_RS_TRANSFER | I2C_TRANSAC_COMP | I2C_HS_NACKERR |
> > I2C_ACKERR,
> > + i2c->base + OFFSET_INTR_STAT);
> > +
> > + /* Enable interrupt */
> > + writew(I2C_RS_TRANSFER | I2C_TRANSAC_COMP, i2c->base +
> > + OFFSET_INTR_MASK);
> > +
> > + writew(I2C_TRAN_DEFAULT_VALUE, i2c->base + OFFSET_TRANSFER_LEN);
> > + writew(I2C_TRANAC_DEFAULT_VALUE, i2c->base + OFFSET_TRANSAC_LEN);
> > +
> > + writew(I2C_TRANSAC_START | I2C_RS_MUL_CNFG, i2c->base +
> > OFFSET_START);
> > +
> > + ret = wait_for_completion_timeout(&i2c->msg_complete,
> > + i2c->adap.timeout);
>
> How does the hardware know that this transaction should be a "master code"?
> Do you have to tell the hardware what value ('1XXX') to use as the
> master code?
When we set i2c speed > 400K , the hardware will send master code. We
have a register to configure master code，and usually we use the default
value "1000".
> The Master Code must be sent at <= 400 kHz, not the target clock. How
> does the hardware know what rate to use?
The i2c speed is defined in dts, and we will set the i2c speed in i2c
register, so HW will know the rate to use.
> When sending the master code, arbitration is supposed to occur, such
> that only one winning master can proceed with the following high speed
> transaction.
> Where do you check that you won this arbitration?
> If this is not implemented, adding a "TODO" would be helpful.
>
Our i2c driver now only support one master.
> > +
> > + completion_done(&i2c->msg_complete);
>
> This completion_done() is only useful if you check the return value.
> You should check it too, since we should only check for timeout if the
> message hasn't completed.
>
I will delete the completion_done() function.
> > +
> > + if (ret == 0) {
> > + dev_dbg(i2c->dev, "send master code timeout.\n");
> > + mtk_i2c_init_hw(i2c);
> > + return -ETIMEDOUT;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> >int num, int left_num)
> > {
> > @@ -539,6 +578,12 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
> > }
> > }
> >
> > + if (i2c->auto_restart && i2c->speed_hz > 40) {
>
> Don't we need to send the master code for *every* HS transaction, not
> just "auto_restart"?
>
When we don't use auto restart, we also need to send the master code,
the master code is sent by HW automatically.
I am improving the mas
Re: [PATCH v2] arm64: dts: add all hi6220 i2c nodes
hi Shawn
On 2015/12/2 10:05, Shawn Guo wrote:
> On Thu, Nov 26, 2015 at 03:57:03PM +0800, Xinwei Kong wrote:
>> This patch adds all I2C nodes for the Hi6220 SoC. This hi6220 Soc
>> use this I2C IP of Synopsys Designware for HiKey board.
>>
>> Signed-off-by: Xinwei Kong
>> ---
>> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 37
>> +++
>> 1 file changed, 37 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> index 82d2488..85d4a8b 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> @@ -208,5 +208,42 @@
>> clock-names = "uartclk", "apb_pclk";
>> status = "disabled";
>> };
>> +
>> +i2c0: i2c@f710 {
>> +compatible = "snps,designware-i2c";
>> +reg = <0x0 0xf710 0x0 0x1000>;
>> +interrupts = <0 44 4>;
>> +clocks = <&sys_ctrl HI6220_I2C0_CLK>;
>> +clock-names = "clk_i2c0";
>
> Neither bindings doc i2c-designware.txt defines this property, nor
> kernel i2c-designware driver uses this property. So I think this
> clock-names property can just be dropped.
>
This property should been delete it. I will debug it in HiKey board then
send v3 patch.
Thank you
xinwei
> Otherwise, the patch looks good to me.
>
> Shawn
>
>> +i2c-sda-hold-time-ns = <300>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>;
>> +status = "disabled";
>> +};
>> +
>> +i2c1: i2c@f7101000 {
>> +compatible = "snps,designware-i2c";
>> +reg = <0x0 0xf7101000 0x0 0x1000>;
>> +interrupts = <0 45 4>;
>> +clocks = <&sys_ctrl HI6220_I2C1_CLK>;
>> +clock-names = "clk_i2c1";
>> +i2c-sda-hold-time-ns = <300>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <&i2c1_pmx_func &i2c1_cfg_func>;
>> +status = "disabled";
>> +};
>> +
>> +i2c2: i2c@f7102000 {
>> +compatible = "snps,designware-i2c";
>> +reg = <0x0 0xf7102000 0x0 0x1000>;
>> +interrupts = <0 46 4>;
>> +clocks = <&sys_ctrl HI6220_I2C2_CLK>;
>> +clock-names = "clk_i2c2";
>> +i2c-sda-hold-time-ns = <300>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <&i2c2_pmx_func &i2c2_cfg_func>;
>> +status = "disabled";
>> +};
>> +
>> };
>> };
>> --
>> 1.9.1
>>
>>
>>
>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] arm64: dts: add all hi6220 i2c nodes
On Thu, Nov 26, 2015 at 03:57:03PM +0800, Xinwei Kong wrote:
> This patch adds all I2C nodes for the Hi6220 SoC. This hi6220 Soc
> use this I2C IP of Synopsys Designware for HiKey board.
>
> Signed-off-by: Xinwei Kong
> ---
> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 37
> +++
> 1 file changed, 37 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index 82d2488..85d4a8b 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -208,5 +208,42 @@
> clock-names = "uartclk", "apb_pclk";
> status = "disabled";
> };
> +
> + i2c0: i2c@f710 {
> + compatible = "snps,designware-i2c";
> + reg = <0x0 0xf710 0x0 0x1000>;
> + interrupts = <0 44 4>;
> + clocks = <&sys_ctrl HI6220_I2C0_CLK>;
> + clock-names = "clk_i2c0";
Neither bindings doc i2c-designware.txt defines this property, nor
kernel i2c-designware driver uses this property. So I think this
clock-names property can just be dropped.
Otherwise, the patch looks good to me.
Shawn
> + i2c-sda-hold-time-ns = <300>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pmx_func &i2c0_cfg_func>;
> + status = "disabled";
> + };
> +
> + i2c1: i2c@f7101000 {
> + compatible = "snps,designware-i2c";
> + reg = <0x0 0xf7101000 0x0 0x1000>;
> + interrupts = <0 45 4>;
> + clocks = <&sys_ctrl HI6220_I2C1_CLK>;
> + clock-names = "clk_i2c1";
> + i2c-sda-hold-time-ns = <300>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pmx_func &i2c1_cfg_func>;
> + status = "disabled";
> + };
> +
> + i2c2: i2c@f7102000 {
> + compatible = "snps,designware-i2c";
> + reg = <0x0 0xf7102000 0x0 0x1000>;
> + interrupts = <0 46 4>;
> + clocks = <&sys_ctrl HI6220_I2C2_CLK>;
> + clock-names = "clk_i2c2";
> + i2c-sda-hold-time-ns = <300>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c2_pmx_func &i2c2_cfg_func>;
> + status = "disabled";
> + };
> +
> };
> };
> --
> 1.9.1
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 16/16] i2c: designware: Convert to use unified device property API
On Tuesday, December 01, 2015 12:33:51 PM Andy Shevchenko wrote: > On Mon, 2015-11-30 at 20:58 +0100, Wolfram Sang wrote: > > On Mon, Nov 30, 2015 at 05:11:44PM +0200, Andy Shevchenko wrote: > > > From: Mika Westerberg > > > > > > With ACPI _DSD (introduced in ACPI v5.1) it is now possible to pass > > > device > > > configuration information from ACPI in addition to DT. In order to > > > support > > > this, convert the driver to use the unified device property > > > accessors > > > instead of DT specific. > > > > > > Change to ordering a bit so that we first try platform data and if > > > that's > > > not available look from device properties. ACPI *CNT methods are > > > then used > > > as last resort to override everything else. > > > > > > Signed-off-by: Mika Westerberg > > > Signed-off-by: Andy Shevchenko > > > Acked-by: Jarkko Nikula > > > > What is the bug fix here described in the cover letter? > > The cover letter mentioned 'last part' which I refer to as patches 14, > 15 (though this is for UART), and 16. Hmm. So may I assume that patches [1-13/16] are for me and the rest is to be applied by the other respective maintainers? That should be easiest logistically IMHO. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: piix4: remove unneeded assignments
Hi Wolfram, On 30.11.2015 14:43, Wolfram Sang wrote: > smatch rightfully says: > drivers/i2c/busses/i2c-piix4.c:504 piix4_access warn: unused return: i = > inb_p() > drivers/i2c/busses/i2c-piix4.c:537 piix4_access warn: unused return: i = > inb_p() > > Signed-off-by: Wolfram Sang > --- > > Christian, can you please check on HW? I could only compile-test. Your patch works fine on my machine! Thanks for merging my patchset btw. Christian > > drivers/i2c/busses/i2c-piix4.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c > index af4e606f8886bd..e045985950737c 100644 > --- a/drivers/i2c/busses/i2c-piix4.c > +++ b/drivers/i2c/busses/i2c-piix4.c > @@ -501,7 +501,7 @@ static s32 piix4_access(struct i2c_adapter * adap, u16 > addr, > if (len == 0 || len > I2C_SMBUS_BLOCK_MAX) > return -EINVAL; > outb_p(len, SMBHSTDAT0); > - i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */ > + inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */ > for (i = 1; i <= len; i++) > outb_p(data->block[i], SMBBLKDAT); > } > @@ -534,7 +534,7 @@ static s32 piix4_access(struct i2c_adapter * adap, u16 > addr, > data->block[0] = inb_p(SMBHSTDAT0); > if (data->block[0] == 0 || data->block[0] > I2C_SMBUS_BLOCK_MAX) > return -EPROTO; > - i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */ > + inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */ > for (i = 1; i <= data->block[0]; i++) > data->block[i] = inb_p(SMBBLKDAT); > break; > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt
On Tue, Dec 01, 2015 at 01:25:50PM +0100, Christophe Ricard wrote: >For example during an i2c_device_probe where an i2c slave device >describe in devicetree has an interrupts property. >i2c_device_probe (drivers/i2c/i2c-core.c), retrieves irq property from >of_irq_get which will looks for an "interrupts" property in >of_irq_parse_of (drivers/of/irq.c). >of_irq_get will then call irq_create_mapping (kernel/irq/irqdomain.c) >which will set the irq_type retrieved during the interrupts node >parsing. Found it now thanks. >This will allow from an i2c slave drivers to configure an interrupt >handler matching the exact devicetree data for the interrupts property >of the i2c slave node. Makes sense. >Now for the same kind of i2c driver using acpi description, the GpioInt >polarity/type is at the moment never kept in the irq property. >It is possible to check that following about the same path... >i2c_device_probe (drivers/i2c/i2c-core.c), retrieves irq property from >acpi_dev_gpio_irq_get but does not save the irq_type. >This would allow not to have to use an additional gpio field and all >the configuration step to configure the gpio interrupt correctly in a >device driver and taking a real benefit of the GpioInt acpi keyword >compare to GpioIo keyword. >Most the of the drivers based on acpi description retrieve gpio number >to assign an interrupt and a fix polarity. I believe my patchset >proposal would improve this and allow to >be much closer with devicetree. >Do you see any issue with this ? No, but I wonder if it would be better to do this in acpi_dev_gpio_irq_get() instead of acpi_find_gpio() which gets called everytime a GPIO is looked up? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 2/2] i2c: mediatek: fix i2c multi transfer issue in high speed mode
Dear Daniel,
1. I will rename "TRAN" & "TRANAC" & "I2C_TRAN_DEFAULT_VALUE" &
"I2C_TRAN_DEFAULT_VALUE".
2. How does the hardware know that this transaction should be a "master code"?
When we set i2c speed > 400K , the hardware will send master code. We have a
register to configure master code，and usually we use the default value
"1000".
How does the hardware know what rate to use?
The i2c speed is defined in dts, and we will set the i2c speed in i2c register,
so HW will know the rate to use.
When sending the master code, arbitration is supposed to occur, such that only
one winning master can proceed with the following high speed transaction.
Where do you check that you won this arbitration?
Our i2c driver now only support one master.
3. I will delete the completion_done() function.
4. Don't we need to send the master code for *every* HS transaction, not just
"auto_restart"?
When we don't use auto restart, we also need to send the master code, the
master code is sent by HW automatically.
I am improving the master code section when use auto restart, and this will be
released in the next patch.
"40" => You already have a macro for this: MAX_FS_MODE_SPEED
Yes, I will use MAX_FS_MODE_SPEED.
Thanks!
-Original Message-
From: djku...@google.com [mailto:djku...@google.com] On Behalf Of Daniel Kurtz
Sent: 2015年11月14日 22:39
To: Liguo Zhang (张立国)
Cc: Wolfram Sang; srv_heupstream; Matthias Brugger; Eddie Huang (黃智傑); Xudong
Chen (陈旭东); Sascha Hauer; Linux I2C; linux-ker...@vger.kernel.org;
linux-arm-ker...@lists.infradead.org; linux-media...@lists.infradead.org
Subject: Re: [PATCH v2 2/2] i2c: mediatek: fix i2c multi transfer issue in high
speed mode
On Mon, Nov 9, 2015 at 1:43 PM, Liguo Zhang wrote:
> For platform with auto restart support, when doing i2c multi transfer
> in high speed, for example, doing write-then-read transfer, the master
> code will occupy the first transfer, and the second transfer will be
> the read transfer, the write transfer will be discarded. So we should
> first send the master code, and then start i2c multi transfer.
>
> Signed-off-by: Liguo Zhang
> Reviewed-by: Eddie Huang
> ---
> drivers/i2c/busses/i2c-mt65xx.c | 45
> +
> 1 file changed, 45 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-mt65xx.c
> b/drivers/i2c/busses/i2c-mt65xx.c index dc4aac6..249df86 100644
> --- a/drivers/i2c/busses/i2c-mt65xx.c
> +++ b/drivers/i2c/busses/i2c-mt65xx.c
> @@ -53,6 +53,8 @@
> #define I2C_FS_TIME_INIT_VALUE 0x1303
> #define I2C_WRRD_TRANAC_VALUE 0x0002
> #define I2C_RD_TRANAC_VALUE0x0001
> +#define I2C_TRAN_DEFAULT_VALUE 0x0001
> +#define I2C_TRAN_DEFAULT_VALUE 0x0001
"TRAN" and "TRANAC" are not good names; this should be "TRANSFER_LEN"
and "TRANSAC", based on the names of the registers to which you write these
constants.
Furthermore, these are not "default" values, they are the transfer length and
number of transactions for sending the "master code", so:
#define I2C_TRANSFER_LEN_MASTER_CODE 0x0001
#define I2C_TRANSAC_LEN_MASTER_CODE 0x0001
Similarly, I think the "TRANAC" in I2C_WRRD_TRANAC_VALUE and
I2C_RD_TRANAC_VALUE should also be TRANSAC.
>
> #define I2C_DMA_CON_TX 0x
> #define I2C_DMA_CON_RX 0x0001
> @@ -365,6 +367,43 @@ static int mtk_i2c_set_speed(struct mtk_i2c *i2c,
> unsigned int parent_clk,
> return 0;
> }
>
> +static int mtk_i2c_send_master_code(struct mtk_i2c *i2c) {
> + int ret = 0;
> +
> + reinit_completion(&i2c->msg_complete);
> +
> + writew(I2C_CONTROL_RS | I2C_CONTROL_ACKERR_DET_EN |
> + I2C_CONTROL_CLK_EXT_EN | I2C_CONTROL_DMA_EN,
> + i2c->base + OFFSET_CONTROL);
> +
> + /* Clear interrupt status */
> + writew(I2C_RS_TRANSFER | I2C_TRANSAC_COMP | I2C_HS_NACKERR |
> I2C_ACKERR,
> + i2c->base + OFFSET_INTR_STAT);
> +
> + /* Enable interrupt */
> + writew(I2C_RS_TRANSFER | I2C_TRANSAC_COMP, i2c->base +
> + OFFSET_INTR_MASK);
> +
> + writew(I2C_TRAN_DEFAULT_VALUE, i2c->base + OFFSET_TRANSFER_LEN);
> + writew(I2C_TRANAC_DEFAULT_VALUE, i2c->base +
> + OFFSET_TRANSAC_LEN);
> +
> + writew(I2C_TRANSAC_START | I2C_RS_MUL_CNFG, i2c->base +
> + OFFSET_START);
> +
> + ret = wait_for_completion_timeout(&i2c->msg_complete,
> + i2c->adap.timeout);
How does the hardware know that this transaction should be a "master code"?
Do you have to tell the hardware what value ('1XXX') to use as the master
code?
The Master Code must be sent at <= 400 kHz, not the target clock. How does the
hardware know what rate to use?
When sending the master code, arbitration is supposed to occur, such that only
one winning master can proceed with the following high speed transaction.
Where do you check that you won this arbitration?
If this is not imp
Re: [PATCH v3 0/2] ACPI: Add irq_type to gpio interrupt
On Mon, Nov 30, 2015 at 11:47:51PM +0100, Christophe Ricard wrote: > ACPI probing method does not retrieve irq_type from a gpio interrupt declared > with GpioInt as it is done with devicetree probing. In other terms, > irq_get_trigger_type > will always send back 0. Is this real problem you are solving here? Also where does the device tree version set triggering flags for a GPIO irq? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 16/16] i2c: designware: Convert to use unified device property API
On Mon, 2015-11-30 at 20:58 +0100, Wolfram Sang wrote: > On Mon, Nov 30, 2015 at 05:11:44PM +0200, Andy Shevchenko wrote: > > From: Mika Westerberg > > > > With ACPI _DSD (introduced in ACPI v5.1) it is now possible to pass > > device > > configuration information from ACPI in addition to DT. In order to > > support > > this, convert the driver to use the unified device property > > accessors > > instead of DT specific. > > > > Change to ordering a bit so that we first try platform data and if > > that's > > not available look from device properties. ACPI *CNT methods are > > then used > > as last resort to override everything else. > > > > Signed-off-by: Mika Westerberg > > Signed-off-by: Andy Shevchenko > > Acked-by: Jarkko Nikula > > What is the bug fix here described in the cover letter? The cover letter mentioned 'last part' which I refer to as patches 14, 15 (though this is for UART), and 16. -- Andy Shevchenko Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 16/16] i2c: designware: Convert to use unified device property API
On Mon, Nov 30, 2015 at 08:58:58PM +0100, Wolfram Sang wrote: > On Mon, Nov 30, 2015 at 05:11:44PM +0200, Andy Shevchenko wrote: > > From: Mika Westerberg > > > > With ACPI _DSD (introduced in ACPI v5.1) it is now possible to pass device > > configuration information from ACPI in addition to DT. In order to support > > this, convert the driver to use the unified device property accessors > > instead of DT specific. > > > > Change to ordering a bit so that we first try platform data and if that's > > not available look from device properties. ACPI *CNT methods are then used > > as last resort to override everything else. > > > > Signed-off-by: Mika Westerberg > > Signed-off-by: Andy Shevchenko > > Acked-by: Jarkko Nikula > > What is the bug fix here described in the cover letter? The bug fix is actually in patch [14/16] "mfd: intel-lpss: Pass SDA hold time to I2C host controller driver". > And shall this go via I2C or via the rest of the series? Either way works. This should compile and work fine without the rest but of course fix for the issue with Lenovo Yoga 900 toucpad still requires all the patches. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
