From: Raviteja Narayanam <rna@xilinx.com> To: Marek Vasut <marex@denx.de>, "linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>, Michal Simek <michals@xilinx.com> Cc: "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, git <git@xilinx.com> Subject: RE: [PATCH 02/10] i2c: xiic: Add standard mode support for > 255 byte read transfers Date: Tue, 8 Jun 2021 09:46:37 +0000 [thread overview] Message-ID: <SN6PR02MB4093D109F1E5545318F7E796CA379@SN6PR02MB4093.namprd02.prod.outlook.com> (raw) In-Reply-To: <df5d8f33-1fe0-5760-b19d-300c1f99344e@denx.de> > -----Original Message----- > From: Marek Vasut <marex@denx.de> > Sent: Thursday, June 3, 2021 3:27 PM > To: Raviteja Narayanam <rna@xilinx.com>; linux-i2c@vger.kernel.org; Michal > Simek <michals@xilinx.com> > Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; git > <git@xilinx.com> > Subject: Re: [PATCH 02/10] i2c: xiic: Add standard mode support for > 255 > byte read transfers > > On 6/3/21 7:33 AM, Raviteja Narayanam wrote: > [...] > >>> + if (i2c->dynamic) { > >>> + u8 bytes; > >>> + u16 val; > >>> + > >>> + /* Clear and enable Rx full interrupt. */ > >>> + xiic_irq_clr_en(i2c, XIIC_INTR_RX_FULL_MASK | > >>> + XIIC_INTR_TX_ERROR_MASK); > >>> + > >>> + /* > >>> + * We want to get all but last byte, because the TX_ERROR > >> IRQ > >>> + * is used to indicate error ACK on the address, and > >>> + * negative ack on the last received byte, so to not mix > >>> + * them receive all but last. > >>> + * In the case where there is only one byte to receive > >>> + * we can check if ERROR and RX full is set at the same time > >>> + */ > >>> + rx_watermark = msg->len; > >>> + bytes = min_t(u8, rx_watermark, IIC_RX_FIFO_DEPTH); > >>> + bytes--; > >>> + > >>> + xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, bytes); > >>> + > >>> + local_irq_save(flags); > >>> + if (!(msg->flags & I2C_M_NOSTART)) > >>> + /* write the address */ > >>> + xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, > >>> + i2c_8bit_addr_from_msg(msg) | > >>> + XIIC_TX_DYN_START_MASK); > >>> + > >>> + xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK); > >>> + > >>> + /* If last message, include dynamic stop bit with length */ > >>> + val = (i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0; > >>> + val |= msg->len; > >>> + > >>> + xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, val); > >>> + local_irq_restore(flags); > >> > >> Is local_irq_save()/local_irq_restore() used here to prevent > >> concurrent access to the XIIC ? > > > > These were used to fix the timing constraint between two register writes > to the IP. > > As we have discussed last time, these are removed in " [PATCH 08/10] i2c: > xiic: Remove interrupt enable/disable in Rx path". > > Now they are no longer needed since our IP is fixed. > > For legacy IP versions, driver is switching to 'AXI I2C standard mode' > > of operation in " [PATCH 07/10] i2c: xiic: Switch to Xiic standard mode for > i2c-read". > > I see, I would expect such fixes to be at the beginning of the series, so they > can be picked into linux-stable. Yes, I will send a v2 with only fixes by removing 0004, 0005, 0006, 0009 and 0010 patches. I can send these five separately afterwards. > > In fact, now that you mention the bugfixes which I posted a year ago, they > also fixed a multitude of locking issues on SMP in the xiic driver. This series addressed the issue found in IP about the timing constraint in register writes. Removed the usage of local_irq_save/restore and gave fix (xiic standard mode) for the affected IP versions. All the i2c app requests are serialized through the i2c core into the driver. > Has this series been tested on SMP Zynq or ZynqMP extensively ? Yes, we have tested this on Zynqmp with multiple applications running, and no issues are observed. Regards, Raviteja N
WARNING: multiple messages have this Message-ID (diff)
From: Raviteja Narayanam <rna@xilinx.com> To: Marek Vasut <marex@denx.de>, "linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>, Michal Simek <michals@xilinx.com> Cc: "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, git <git@xilinx.com> Subject: RE: [PATCH 02/10] i2c: xiic: Add standard mode support for > 255 byte read transfers Date: Tue, 8 Jun 2021 09:46:37 +0000 [thread overview] Message-ID: <SN6PR02MB4093D109F1E5545318F7E796CA379@SN6PR02MB4093.namprd02.prod.outlook.com> (raw) In-Reply-To: <df5d8f33-1fe0-5760-b19d-300c1f99344e@denx.de> > -----Original Message----- > From: Marek Vasut <marex@denx.de> > Sent: Thursday, June 3, 2021 3:27 PM > To: Raviteja Narayanam <rna@xilinx.com>; linux-i2c@vger.kernel.org; Michal > Simek <michals@xilinx.com> > Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; git > <git@xilinx.com> > Subject: Re: [PATCH 02/10] i2c: xiic: Add standard mode support for > 255 > byte read transfers > > On 6/3/21 7:33 AM, Raviteja Narayanam wrote: > [...] > >>> + if (i2c->dynamic) { > >>> + u8 bytes; > >>> + u16 val; > >>> + > >>> + /* Clear and enable Rx full interrupt. */ > >>> + xiic_irq_clr_en(i2c, XIIC_INTR_RX_FULL_MASK | > >>> + XIIC_INTR_TX_ERROR_MASK); > >>> + > >>> + /* > >>> + * We want to get all but last byte, because the TX_ERROR > >> IRQ > >>> + * is used to indicate error ACK on the address, and > >>> + * negative ack on the last received byte, so to not mix > >>> + * them receive all but last. > >>> + * In the case where there is only one byte to receive > >>> + * we can check if ERROR and RX full is set at the same time > >>> + */ > >>> + rx_watermark = msg->len; > >>> + bytes = min_t(u8, rx_watermark, IIC_RX_FIFO_DEPTH); > >>> + bytes--; > >>> + > >>> + xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, bytes); > >>> + > >>> + local_irq_save(flags); > >>> + if (!(msg->flags & I2C_M_NOSTART)) > >>> + /* write the address */ > >>> + xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, > >>> + i2c_8bit_addr_from_msg(msg) | > >>> + XIIC_TX_DYN_START_MASK); > >>> + > >>> + xiic_irq_clr_en(i2c, XIIC_INTR_BNB_MASK); > >>> + > >>> + /* If last message, include dynamic stop bit with length */ > >>> + val = (i2c->nmsgs == 1) ? XIIC_TX_DYN_STOP_MASK : 0; > >>> + val |= msg->len; > >>> + > >>> + xiic_setreg16(i2c, XIIC_DTR_REG_OFFSET, val); > >>> + local_irq_restore(flags); > >> > >> Is local_irq_save()/local_irq_restore() used here to prevent > >> concurrent access to the XIIC ? > > > > These were used to fix the timing constraint between two register writes > to the IP. > > As we have discussed last time, these are removed in " [PATCH 08/10] i2c: > xiic: Remove interrupt enable/disable in Rx path". > > Now they are no longer needed since our IP is fixed. > > For legacy IP versions, driver is switching to 'AXI I2C standard mode' > > of operation in " [PATCH 07/10] i2c: xiic: Switch to Xiic standard mode for > i2c-read". > > I see, I would expect such fixes to be at the beginning of the series, so they > can be picked into linux-stable. Yes, I will send a v2 with only fixes by removing 0004, 0005, 0006, 0009 and 0010 patches. I can send these five separately afterwards. > > In fact, now that you mention the bugfixes which I posted a year ago, they > also fixed a multitude of locking issues on SMP in the xiic driver. This series addressed the issue found in IP about the timing constraint in register writes. Removed the usage of local_irq_save/restore and gave fix (xiic standard mode) for the affected IP versions. All the i2c app requests are serialized through the i2c core into the driver. > Has this series been tested on SMP Zynq or ZynqMP extensively ? Yes, we have tested this on Zynqmp with multiple applications running, and no issues are observed. Regards, Raviteja N _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-06-08 9:46 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-31 13:19 [PATCH 00/10] i2c: xiic: Add features, bug fixes Raviteja Narayanam 2021-05-31 13:19 ` Raviteja Narayanam 2021-05-31 13:19 ` [PATCH 01/10] i2c: xiic: Fix Tx Interrupt path for grouped messages Raviteja Narayanam 2021-05-31 13:19 ` Raviteja Narayanam 2021-05-31 13:19 ` [PATCH 02/10] i2c: xiic: Add standard mode support for > 255 byte read transfers Raviteja Narayanam 2021-05-31 13:19 ` Raviteja Narayanam 2021-06-02 11:17 ` Marek Vasut 2021-06-02 11:17 ` Marek Vasut 2021-06-03 5:33 ` Raviteja Narayanam 2021-06-03 5:33 ` Raviteja Narayanam 2021-06-03 9:57 ` Marek Vasut 2021-06-03 9:57 ` Marek Vasut 2021-06-08 9:46 ` Raviteja Narayanam [this message] 2021-06-08 9:46 ` Raviteja Narayanam 2021-05-31 13:19 ` [PATCH 03/10] i2c: xiic: Fix coding style issues Raviteja Narayanam 2021-05-31 13:19 ` Raviteja Narayanam 2021-05-31 15:49 ` Joe Perches 2021-05-31 15:49 ` Joe Perches 2021-05-31 13:19 ` [PATCH 04/10] i2c: xiic: Add smbus_block_read functionality Raviteja Narayanam 2021-05-31 13:19 ` Raviteja Narayanam 2021-05-31 13:19 ` [PATCH 05/10] i2c: xiic: Return value of xiic_reinit Raviteja Narayanam 2021-05-31 13:19 ` Raviteja Narayanam 2021-05-31 13:19 ` [PATCH 06/10] i2c: xiic: Fix the type check for xiic_wakeup Raviteja Narayanam 2021-05-31 13:19 ` Raviteja Narayanam 2021-05-31 13:19 ` [PATCH 07/10] i2c: xiic: Switch to Xiic standard mode for i2c-read Raviteja Narayanam 2021-05-31 13:19 ` Raviteja Narayanam 2021-05-31 13:19 ` [PATCH 08/10] i2c: xiic: Remove interrupt enable/disable in Rx path Raviteja Narayanam 2021-05-31 13:19 ` Raviteja Narayanam 2021-05-31 13:19 ` [PATCH 09/10] dt-bindings: i2c: xiic: Add 'xlnx,axi-iic-2.1' to compatible Raviteja Narayanam 2021-05-31 13:19 ` [PATCH 09/10] dt-bindings: i2c: xiic: Add 'xlnx, axi-iic-2.1' " Raviteja Narayanam 2021-05-31 13:19 ` [PATCH 10/10] i2c: xiic: Update compatible with new IP version Raviteja Narayanam 2021-05-31 13:19 ` Raviteja Narayanam
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=SN6PR02MB4093D109F1E5545318F7E796CA379@SN6PR02MB4093.namprd02.prod.outlook.com \ --to=rna@xilinx.com \ --cc=git@xilinx.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-i2c@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marex@denx.de \ --cc=michals@xilinx.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.