[PATCH v1 1/1] arm64: dts: stingray: fix usb dma address translation
From: Bharat Gooty
Add a non-empty dma-ranges so that dma address translation
happens.
Fixes: 2013a4b684b6 ("arm64: dts: broadcom: clear the warnings caused by empty
dma-ranges")
Signed-off-by: Bharat Gooty
Signed-off-by: Rayagonda Kokatanur
---
arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi
b/arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi
index aef8f2b00778..5401a646c840 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi
@@ -4,11 +4,16 @@
*/
usb {
compatible = "simple-bus";
- dma-ranges;
#address-cells = <2>;
#size-cells = <2>;
ranges = <0x0 0x0 0x0 0x6850 0x0 0x0040>;
+ /*
+* Internally, USB bus to the interconnect can only address up
+* to 40-bit
+*/
+ dma-ranges = <0 0 0 0 0x100 0x0>;
+
usbphy0: usb-phy@0 {
compatible = "brcm,sr-usb-combo-phy";
reg = <0x0 0x 0x0 0x100>;
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v3 5/6] i2c: iproc: handle master read request
On Fri, Dec 18, 2020 at 12:41 AM Ray Jui wrote: > > > > On 12/16/2020 8:08 PM, Rayagonda Kokatanur wrote: > > On Wed, Dec 2, 2020 at 11:14 PM Ray Jui wrote: > >> > >> > >> > >> On 12/2/2020 6:35 AM, Wolfram Sang wrote: > >>> > >>>> All review comments are scattered now, please let me know what has to be > >>>> done further, > >>>> Are we going to change the tasklet to irq thread ? > >>>> Are we going to remove batching 64 packets if transaction > 64B and use > >>>> rx > >>>> fifo threshold ? > >>>> > >>>> I don't see any issue with current code but if it has to change we need a > >>>> valid reason for the same. > >>>> If nothing to be done, please acknowledge the patch. > >>> > >>> Valid request. Has there been any news? > >>> > >> > >> Sorry for the delay. I just replied. > > > > This patch is tested and validated with all corner cases and its working. > > Can we merge this and take up any improvement as part of separate patch? > > > > I think that makes sense, and I'm okay with these patches going in as > they are now. > > Acked-by: Ray Jui Thank you. > > But please help to collect precise FIFO access timing (later when you > have time), that would allow us to know if the current defer-to-tasklet > (instead of thread) based approach makes sense or not. > > Thanks, > > Ray > > > Thanks, > > Rayagonda > > > >> > >> > >> Thanks, > >> > >> Ray -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v3 5/6] i2c: iproc: handle master read request
On Wed, Dec 2, 2020 at 11:14 PM Ray Jui wrote: > > > > On 12/2/2020 6:35 AM, Wolfram Sang wrote: > > > >> All review comments are scattered now, please let me know what has to be > >> done further, > >> Are we going to change the tasklet to irq thread ? > >> Are we going to remove batching 64 packets if transaction > 64B and use rx > >> fifo threshold ? > >> > >> I don't see any issue with current code but if it has to change we need a > >> valid reason for the same. > >> If nothing to be done, please acknowledge the patch. > > > > Valid request. Has there been any news? > > > > Sorry for the delay. I just replied. This patch is tested and validated with all corner cases and its working. Can we merge this and take up any improvement as part of separate patch? Thanks, Rayagonda > > > Thanks, > > Ray -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v3 5/6] i2c: iproc: handle master read request
Hi Ray, Could you please check Dhananjay comments and update your thoughts. On Fri, Nov 6, 2020 at 11:11 PM Dhananjay Phadke wrote: > > On Thu, 5 Nov 2020 15:13:04 +0530, Rayagonda Kokatanur wrote: > >> So the suggestion was to set HW threshold for rx fifo interrupt, not > >> really a SW property. By setting it in DT, makes it easier to > >> customize for target system, module param needs or ioctl makes it > >> dependent on userpsace to configure it. > >> > >> The need for tasklet seems to arise from the fact that many bytes are > >> left in the fifo. If there's a common problem here, such tasklet would be > >> needed in i2c subsys rather than controller specific tweak, akin to > >> how networking uses NAPI or adding block transactions to the interface? > >> > >> For master write-read event, it seems both IS_S_RD_EVENT_SHIFT and > >> IS_S_RX_EVENT_SHIFT are detected, which implies that core is late to > >> drain rx fifo i.e. write is complete and the read has started on the bus? > > > >Yes it's true that for master write-read events both > >IS_S_RD_EVENT_SHIFT and IS_S_RX_EVENT_SHIFT are coming together. > >So before the slave starts transmitting data to the master, it should > >first read all data from rx-fifo i.e. complete master write and then > >process master read. > > > >To minimise interrupt overhead, we are batching 64bytes. > >To keep isr running for less time, we are using a tasklet. > >Again to keep the tasklet not running for more than 20u, we have set > >max of 10 bytes data read from rx-fifo per tasklet run. > > > >If we start processing everything in isr and using rx threshold > >interrupt, then isr will run for a longer time and this may hog the > >system. > >For example, to process 10 bytes it takes 20us, to process 30 bytes it > >takes 60us and so on. > >So is it okay to run isr for so long ? > > > >Keeping all this in mind we thought a tasklet would be a good option > >and kept max of 10 bytes read per tasklet. > > > >Please let me know if you still feel we should not use a tasklet and > >don't batch 64 bytes. > > Deferring to tasklet is OK, could use a kernel thread (i.e. threaded_irq) > as i2c rate is quite low. > > But do enable rx_threshold and read out early. This will avoid fifo full > or master write-read situation where lot of bytes must be drained from rx > fifo before serving tx fifo (avoid tx underrun). > > Best would have been setting up DMA into mem (some controllers seem capable). > In absence of that, it's a trade off: if rx intr threshold is low, > there will be more interrupts, but less time spent in each. Default could > still be 64B or no-thresh (allow override in dtb). > > Few other comments - > > >+ /* schedule tasklet to read data later */ > >+ tasklet_schedule(&iproc_i2c->slave_rx_tasklet); > >+ > >+ /* clear only IS_S_RX_EVENT_SHIFT interrupt */ > >+ iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET, > >+ BIT(IS_S_RX_EVENT_SHIFT)); > >+ } > > Why clearing one rx interrupt bit here after scheduling tasklet? Should all > that > be done by tasklet? Also should just return after scheduling tasklet? > > Thanks, > Dhananjay smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v3 5/6] i2c: iproc: handle master read request
On Thu, Nov 5, 2020 at 1:16 PM Dhananjay Phadke wrote: > > On Wed, 4 Nov 2020 10:01:06 -0800, Ray Jui wrote: > > +#define MAX_SLAVE_RX_PER_INT 10 > > >>> > In patch [3/6], you've enabled IS_S_RX_THLD_SHIFT in slave ISR bitmask, > however it's not actually used in processing rx events. > > Instead of hardcoding this threshold here, it's better to add a > device-tree knob for rx threshold, program it in controller and handle > that RX_THLD interrupt. This will give more flexibility to drain the rx > fifo earlier than - > (1) waiting for FIFO_FULL interrupt for transactions > 64B. > (2) waiting for start of read transaction in case of master write-read. > >> > >> Yes this is one way to implement. > >> But do you see any issue in batching 64 bytes at a time in case of > >> transaction > 64 Bytes. > >> I feel batching will be more efficient as it avoids more number of > >> interrupts and hence context switch. > >> > >>> > >>> The Device Tree is really intended to describe the hardware FIFO size, > >>> not watermarks, as those tend to be more of a policy/work load decision. > >>> Maybe this is something that can be added as a module parameter, or > >>> configurable via ioctl() at some point. > >> > > > >Yes, DT can have properties to describe the FIFO size, if there happens > >to be some variants in the HW blocks in different versions. But that is > >not the case here. DT should not be used to control SW/use case specific > >behavior. > > So the suggestion was to set HW threshold for rx fifo interrupt, not > really a SW property. By setting it in DT, makes it easier to > customize for target system, module param needs or ioctl makes it > dependent on userpsace to configure it. > > The need for tasklet seems to arise from the fact that many bytes are > left in the fifo. If there's a common problem here, such tasklet would be > needed in i2c subsys rather than controller specific tweak, akin to > how networking uses NAPI or adding block transactions to the interface? > > For master write-read event, it seems both IS_S_RD_EVENT_SHIFT and > IS_S_RX_EVENT_SHIFT are detected, which implies that core is late to > drain rx fifo i.e. write is complete and the read has started on the bus? Yes it's true that for master write-read events both IS_S_RD_EVENT_SHIFT and IS_S_RX_EVENT_SHIFT are coming together. So before the slave starts transmitting data to the master, it should first read all data from rx-fifo i.e. complete master write and then process master read. To minimise interrupt overhead, we are batching 64bytes. To keep isr running for less time, we are using a tasklet. Again to keep the tasklet not running for more than 20u, we have set max of 10 bytes data read from rx-fifo per tasklet run. If we start processing everything in isr and using rx threshold interrupt, then isr will run for a longer time and this may hog the system. For example, to process 10 bytes it takes 20us, to process 30 bytes it takes 60us and so on. So is it okay to run isr for so long ? Keeping all this in mind we thought a tasklet would be a good option and kept max of 10 bytes read per tasklet. Please let me know if you still feel we should not use a tasklet and don't batch 64 bytes. Thanks Rayagonda > > > Thanks, > Dhananjay > > smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v3 5/6] i2c: iproc: handle master read request
On Wed, Nov 4, 2020 at 9:05 AM Florian Fainelli wrote:
>
>
>
> On 11/2/2020 10:19 PM, Dhananjay Phadke wrote:
> > On Mon, 2 Nov 2020 09:24:32 +0530, Rayagonda Kokatanur wrote:
> >
> >> Handle single or multi byte master read request with or without
> >> repeated start.
> >>
> >> Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for
> >> slave mode")
> >> Signed-off-by: Rayagonda Kokatanur
> >> ---
> >> drivers/i2c/busses/i2c-bcm-iproc.c | 215 +++--
> >> 1 file changed, 170 insertions(+), 45 deletions(-)
> >>
> >> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
> >> b/drivers/i2c/busses/i2c-bcm-iproc.c
> >> index 7a235f9f5884..22e04055b447 100644
> >> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> >> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> >> @@ -160,6 +160,11 @@
> >>
> >> #define IE_S_ALL_INTERRUPT_SHIFT 21
> >> #define IE_S_ALL_INTERRUPT_MASK 0x3f
> >> +/*
> >> + * It takes ~18us to reading 10bytes of data, hence to keep tasklet
> >> + * running for less time, max slave read per tasklet is set to 10 bytes.
> >> + */
> >> +#define MAX_SLAVE_RX_PER_INT 10
> >>
> >
> > In patch [3/6], you've enabled IS_S_RX_THLD_SHIFT in slave ISR bitmask,
> > however it's not actually used in processing rx events.
> >
> > Instead of hardcoding this threshold here, it's better to add a
> > device-tree knob for rx threshold, program it in controller and handle
> > that RX_THLD interrupt. This will give more flexibility to drain the rx
> > fifo earlier than -
> > (1) waiting for FIFO_FULL interrupt for transactions > 64B.
> > (2) waiting for start of read transaction in case of master write-read.
Yes this is one way to implement.
But do you see any issue in batching 64 bytes at a time in case of
transaction > 64 Bytes.
I feel batching will be more efficient as it avoids more number of
interrupts and hence context switch.
>
> The Device Tree is really intended to describe the hardware FIFO size,
> not watermarks, as those tend to be more of a policy/work load decision.
> Maybe this is something that can be added as a module parameter, or
> configurable via ioctl() at some point.
#define MAX_SLAVE_RX_PER_INT 10
is not hw fifo threshold level, it is a kind of watermark for the
tasklet to process the max number of packets in single run.
The intention to add the macro is to make sure the tasklet does not
run more than 20us.
If we keep this as a module parameter or dt parameter then there is a
good possibility that the number can be set to higher value.
This will make the tasklet run more than 20us and defeat the purpose.
This number is constant and not variable to change
Please feel free to add your comments.
Best regards,
Rayagonda
> --
> Florian
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v3 0/6] fix iproc driver to handle master read request
This series of patches adds the following, - Handle master abort error - Fix support for single/multi byte master read request with/without repeated start. - Handle rx fifo full interrupt - Fix typo Changes from V2: --Add missing Acked-by tag. Changes from V1: --Address review comments from Ray Jui, Remove fixes tag Rayagonda Kokatanur (6): i2c: iproc: handle Master aborted error i2c: iproc: handle only slave interrupts which are enabled i2c: iproc: update slave isr mask (ISR_MASK_SLAVE) i2c: iproc: fix typo in slave_isr function i2c: iproc: handle master read request i2c: iproc: handle rx fifo full interrupt drivers/i2c/busses/i2c-bcm-iproc.c | 254 +++-- 1 file changed, 200 insertions(+), 54 deletions(-) -- 2.17.1 smime.p7s Description: S/MIME Cryptographic Signature
[PATCH v3 1/6] i2c: iproc: handle Master aborted error
Handle Master aborted error by flushing tx and rx fifo
and reinitializing the hw.
Signed-off-by: Rayagonda Kokatanur
Acked-by: Ray Jui
---
drivers/i2c/busses/i2c-bcm-iproc.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index d8295b1c379d..834a98caeada 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -93,6 +93,7 @@
#define S_CMD_STATUS_MASK0x07
#define S_CMD_STATUS_SUCCESS 0x0
#define S_CMD_STATUS_TIMEOUT 0x5
+#define S_CMD_STATUS_MASTER_ABORT0x7
#define IE_OFFSET0x38
#define IE_M_RX_FIFO_FULL_SHIFT 31
@@ -311,9 +312,10 @@ static void bcm_iproc_i2c_check_slave_status(
return;
val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
- if (val == S_CMD_STATUS_TIMEOUT) {
- dev_err(iproc_i2c->device, "slave random stretch time
timeout\n");
-
+ if (val == S_CMD_STATUS_TIMEOUT || val == S_CMD_STATUS_MASTER_ABORT) {
+ dev_err(iproc_i2c->device, (val == S_CMD_STATUS_TIMEOUT) ?
+ "slave random stretch time timeout\n" :
+ "Master aborted read transaction\n");
/* re-initialize i2c for recovery */
bcm_iproc_i2c_enable_disable(iproc_i2c, false);
bcm_iproc_i2c_slave_init(iproc_i2c, true);
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v3 4/6] i2c: iproc: fix typo in slave_isr function
Fix typo in bcm_iproc_i2c_slave_isr().
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index cd687696bf0b..7a235f9f5884 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -382,7 +382,7 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
if (status & BIT(IS_S_START_BUSY_SHIFT)) {
i2c_slave_event(iproc_i2c->slave, I2C_SLAVE_STOP, &value);
/*
-* Enable interrupt for TX FIFO becomes empty and
+* Disable interrupt for TX FIFO becomes empty and
* less than PKT_LENGTH bytes were output on the SMBUS
*/
val = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v3 6/6] i2c: iproc: handle rx fifo full interrupt
Add code to handle IS_S_RX_FIFO_FULL_SHIFT interrupt to support
master write request with >= 64 bytes.
Iproc has a slave rx fifo size of 64 bytes.
Rx fifo full interrupt (IS_S_RX_FIFO_FULL_SHIFT) will be generated
when RX fifo becomes full. This can happen if master issues write
request of more than 64 bytes.
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 22e04055b447..cceaf69279a9 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -313,6 +313,8 @@ static void bcm_iproc_i2c_slave_init(
/* Enable interrupt register to indicate a valid byte in receive fifo */
val = BIT(IE_S_RX_EVENT_SHIFT);
+ /* Enable interrupt register to indicate Slave Rx FIFO Full */
+ val |= BIT(IE_S_RX_FIFO_FULL_SHIFT);
/* Enable interrupt register to indicate a Master read transaction */
val |= BIT(IE_S_RD_EVENT_SHIFT);
/* Enable interrupt register for the Slave BUSY command */
@@ -434,9 +436,15 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
*events
* Master-read : both IS_S_RX_EVENT_SHIFT and IS_S_RD_EVENT_SHIFT
*events or only IS_S_RD_EVENT_SHIFT
+*
+* iproc has a slave rx fifo size of 64 bytes. Rx fifo full interrupt
+* (IS_S_RX_FIFO_FULL_SHIFT) will be generated when RX fifo becomes
+* full. This can happen if Master issues write requests of more than
+* 64 bytes.
*/
if (status & BIT(IS_S_RX_EVENT_SHIFT) ||
- status & BIT(IS_S_RD_EVENT_SHIFT)) {
+ status & BIT(IS_S_RD_EVENT_SHIFT) ||
+ status & BIT(IS_S_RX_FIFO_FULL_SHIFT)) {
/* disable slave interrupts */
val = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
val &= ~iproc_i2c->slave_int_mask;
@@ -452,9 +460,14 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
/* schedule tasklet to read data later */
tasklet_schedule(&iproc_i2c->slave_rx_tasklet);
- /* clear only IS_S_RX_EVENT_SHIFT interrupt */
- iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET,
-BIT(IS_S_RX_EVENT_SHIFT));
+ /*
+* clear only IS_S_RX_EVENT_SHIFT and
+* IS_S_RX_FIFO_FULL_SHIFT interrupt.
+*/
+ val = BIT(IS_S_RX_EVENT_SHIFT);
+ if (status & BIT(IS_S_RX_FIFO_FULL_SHIFT))
+ val |= BIT(IS_S_RX_FIFO_FULL_SHIFT);
+ iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET, val);
}
if (status & BIT(IS_S_TX_UNDERRUN_SHIFT)) {
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v3 5/6] i2c: iproc: handle master read request
Handle single or multi byte master read request with or without
repeated start.
Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
mode")
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 215 +++--
1 file changed, 170 insertions(+), 45 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 7a235f9f5884..22e04055b447 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -160,6 +160,11 @@
#define IE_S_ALL_INTERRUPT_SHIFT 21
#define IE_S_ALL_INTERRUPT_MASK 0x3f
+/*
+ * It takes ~18us to reading 10bytes of data, hence to keep tasklet
+ * running for less time, max slave read per tasklet is set to 10 bytes.
+ */
+#define MAX_SLAVE_RX_PER_INT 10
enum i2c_slave_read_status {
I2C_SLAVE_RX_FIFO_EMPTY = 0,
@@ -206,8 +211,18 @@ struct bcm_iproc_i2c_dev {
/* bytes that have been read */
unsigned int rx_bytes;
unsigned int thld_bytes;
+
+ bool slave_rx_only;
+ bool rx_start_rcvd;
+ bool slave_read_complete;
+ u32 tx_underrun;
+ u32 slave_int_mask;
+ struct tasklet_struct slave_rx_tasklet;
};
+/* tasklet to process slave rx data */
+static void slave_rx_tasklet_fn(unsigned long);
+
/*
* Can be expanded in the future if more interrupt status bits are utilized
*/
@@ -261,6 +276,7 @@ static void bcm_iproc_i2c_slave_init(
{
u32 val;
+ iproc_i2c->tx_underrun = 0;
if (need_reset) {
/* put controller in reset */
val = iproc_i2c_rd_reg(iproc_i2c, CFG_OFFSET);
@@ -297,8 +313,11 @@ static void bcm_iproc_i2c_slave_init(
/* Enable interrupt register to indicate a valid byte in receive fifo */
val = BIT(IE_S_RX_EVENT_SHIFT);
+ /* Enable interrupt register to indicate a Master read transaction */
+ val |= BIT(IE_S_RD_EVENT_SHIFT);
/* Enable interrupt register for the Slave BUSY command */
val |= BIT(IE_S_START_BUSY_SHIFT);
+ iproc_i2c->slave_int_mask = val;
iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, val);
}
@@ -324,76 +343,176 @@ static void bcm_iproc_i2c_check_slave_status(
}
}
-static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
- u32 status)
+static void bcm_iproc_i2c_slave_read(struct bcm_iproc_i2c_dev *iproc_i2c)
{
+ u8 rx_data, rx_status;
+ u32 rx_bytes = 0;
u32 val;
- u8 value, rx_status;
- /* Slave RX byte receive */
- if (status & BIT(IS_S_RX_EVENT_SHIFT)) {
+ while (rx_bytes < MAX_SLAVE_RX_PER_INT) {
val = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
rx_status = (val >> S_RX_STATUS_SHIFT) & S_RX_STATUS_MASK;
- if (rx_status == I2C_SLAVE_RX_START) {
- /* Start of SMBUS for Master write */
- i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_WRITE_REQUESTED, &value);
+ rx_data = ((val >> S_RX_DATA_SHIFT) & S_RX_DATA_MASK);
- val = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
- value = (u8)((val >> S_RX_DATA_SHIFT) & S_RX_DATA_MASK);
+ if (rx_status == I2C_SLAVE_RX_START) {
+ /* Start of SMBUS Master write */
i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_WRITE_RECEIVED, &value);
- } else if (status & BIT(IS_S_RD_EVENT_SHIFT)) {
- /* Start of SMBUS for Master Read */
+ I2C_SLAVE_WRITE_REQUESTED, &rx_data);
+ iproc_i2c->rx_start_rcvd = true;
+ iproc_i2c->slave_read_complete = false;
+ } else if (rx_status == I2C_SLAVE_RX_DATA &&
+ iproc_i2c->rx_start_rcvd) {
+ /* Middle of SMBUS Master write */
i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_READ_REQUESTED, &value);
- iproc_i2c_wr_reg(iproc_i2c, S_TX_OFFSET, value);
+ I2C_SLAVE_WRITE_RECEIVED, &rx_data);
+ } else if (rx_status == I2C_SLAVE_RX_END &&
+ iproc_i2c->rx_start_rcvd) {
+ /* End of SMBUS Master write */
+ if (iproc_i2c->slave_rx_only)
+ i2c_slave_event(iproc_i2c->slave,
+ I2C_SLAVE_WRITE_RECEIVED,
+ &rx_data);
+
+ i2c_slave_event(iproc_i2c->slave, I2C
[PATCH v3 3/6] i2c: iproc: update slave isr mask (ISR_MASK_SLAVE)
Update slave isr mask (ISR_MASK_SLAVE) to include remaining
two slave interrupts.
Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
mode")
Signed-off-by: Rayagonda Kokatanur
Acked-by: Ray Jui
---
drivers/i2c/busses/i2c-bcm-iproc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index b54f5130d246..cd687696bf0b 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -216,7 +216,8 @@ struct bcm_iproc_i2c_dev {
#define ISR_MASK_SLAVE (BIT(IS_S_START_BUSY_SHIFT)\
| BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT)\
- | BIT(IS_S_TX_UNDERRUN_SHIFT))
+ | BIT(IS_S_TX_UNDERRUN_SHIFT) | BIT(IS_S_RX_FIFO_FULL_SHIFT)\
+ | BIT(IS_S_RX_THLD_SHIFT))
static int bcm_iproc_i2c_reg_slave(struct i2c_client *slave);
static int bcm_iproc_i2c_unreg_slave(struct i2c_client *slave);
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v3 2/6] i2c: iproc: handle only slave interrupts which are enabled
Handle only slave interrupts which are enabled.
The IS_OFFSET register contains the interrupt status bits which will be
set regardless of the enabling of the corresponding interrupt condition.
One must therefore look at both IS_OFFSET and IE_OFFSET to determine
whether an interrupt condition is set and enabled.
Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
mode")
Signed-off-by: Rayagonda Kokatanur
Acked-by: Ray Jui
---
drivers/i2c/busses/i2c-bcm-iproc.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 834a98caeada..b54f5130d246 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -507,12 +507,17 @@ static void bcm_iproc_i2c_process_m_event(struct
bcm_iproc_i2c_dev *iproc_i2c,
static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
{
struct bcm_iproc_i2c_dev *iproc_i2c = data;
- u32 status = iproc_i2c_rd_reg(iproc_i2c, IS_OFFSET);
+ u32 slave_status;
+ u32 status;
bool ret;
- u32 sl_status = status & ISR_MASK_SLAVE;
- if (sl_status) {
- ret = bcm_iproc_i2c_slave_isr(iproc_i2c, sl_status);
+ status = iproc_i2c_rd_reg(iproc_i2c, IS_OFFSET);
+ /* process only slave interrupt which are enabled */
+ slave_status = status & iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET) &
+ ISR_MASK_SLAVE;
+
+ if (slave_status) {
+ ret = bcm_iproc_i2c_slave_isr(iproc_i2c, slave_status);
if (ret)
return IRQ_HANDLED;
else
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v2 4/6] i2c: iproc: fix typo in slave_isr function
Fix typo in bcm_iproc_i2c_slave_isr().
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index cd687696bf0b..7a235f9f5884 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -382,7 +382,7 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
if (status & BIT(IS_S_START_BUSY_SHIFT)) {
i2c_slave_event(iproc_i2c->slave, I2C_SLAVE_STOP, &value);
/*
-* Enable interrupt for TX FIFO becomes empty and
+* Disable interrupt for TX FIFO becomes empty and
* less than PKT_LENGTH bytes were output on the SMBUS
*/
val = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v2 5/6] i2c: iproc: handle master read request
Handle single or multi byte master read request with or without
repeated start.
Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
mode")
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 215 +++--
1 file changed, 170 insertions(+), 45 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 7a235f9f5884..22e04055b447 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -160,6 +160,11 @@
#define IE_S_ALL_INTERRUPT_SHIFT 21
#define IE_S_ALL_INTERRUPT_MASK 0x3f
+/*
+ * It takes ~18us to reading 10bytes of data, hence to keep tasklet
+ * running for less time, max slave read per tasklet is set to 10 bytes.
+ */
+#define MAX_SLAVE_RX_PER_INT 10
enum i2c_slave_read_status {
I2C_SLAVE_RX_FIFO_EMPTY = 0,
@@ -206,8 +211,18 @@ struct bcm_iproc_i2c_dev {
/* bytes that have been read */
unsigned int rx_bytes;
unsigned int thld_bytes;
+
+ bool slave_rx_only;
+ bool rx_start_rcvd;
+ bool slave_read_complete;
+ u32 tx_underrun;
+ u32 slave_int_mask;
+ struct tasklet_struct slave_rx_tasklet;
};
+/* tasklet to process slave rx data */
+static void slave_rx_tasklet_fn(unsigned long);
+
/*
* Can be expanded in the future if more interrupt status bits are utilized
*/
@@ -261,6 +276,7 @@ static void bcm_iproc_i2c_slave_init(
{
u32 val;
+ iproc_i2c->tx_underrun = 0;
if (need_reset) {
/* put controller in reset */
val = iproc_i2c_rd_reg(iproc_i2c, CFG_OFFSET);
@@ -297,8 +313,11 @@ static void bcm_iproc_i2c_slave_init(
/* Enable interrupt register to indicate a valid byte in receive fifo */
val = BIT(IE_S_RX_EVENT_SHIFT);
+ /* Enable interrupt register to indicate a Master read transaction */
+ val |= BIT(IE_S_RD_EVENT_SHIFT);
/* Enable interrupt register for the Slave BUSY command */
val |= BIT(IE_S_START_BUSY_SHIFT);
+ iproc_i2c->slave_int_mask = val;
iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, val);
}
@@ -324,76 +343,176 @@ static void bcm_iproc_i2c_check_slave_status(
}
}
-static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
- u32 status)
+static void bcm_iproc_i2c_slave_read(struct bcm_iproc_i2c_dev *iproc_i2c)
{
+ u8 rx_data, rx_status;
+ u32 rx_bytes = 0;
u32 val;
- u8 value, rx_status;
- /* Slave RX byte receive */
- if (status & BIT(IS_S_RX_EVENT_SHIFT)) {
+ while (rx_bytes < MAX_SLAVE_RX_PER_INT) {
val = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
rx_status = (val >> S_RX_STATUS_SHIFT) & S_RX_STATUS_MASK;
- if (rx_status == I2C_SLAVE_RX_START) {
- /* Start of SMBUS for Master write */
- i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_WRITE_REQUESTED, &value);
+ rx_data = ((val >> S_RX_DATA_SHIFT) & S_RX_DATA_MASK);
- val = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
- value = (u8)((val >> S_RX_DATA_SHIFT) & S_RX_DATA_MASK);
+ if (rx_status == I2C_SLAVE_RX_START) {
+ /* Start of SMBUS Master write */
i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_WRITE_RECEIVED, &value);
- } else if (status & BIT(IS_S_RD_EVENT_SHIFT)) {
- /* Start of SMBUS for Master Read */
+ I2C_SLAVE_WRITE_REQUESTED, &rx_data);
+ iproc_i2c->rx_start_rcvd = true;
+ iproc_i2c->slave_read_complete = false;
+ } else if (rx_status == I2C_SLAVE_RX_DATA &&
+ iproc_i2c->rx_start_rcvd) {
+ /* Middle of SMBUS Master write */
i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_READ_REQUESTED, &value);
- iproc_i2c_wr_reg(iproc_i2c, S_TX_OFFSET, value);
+ I2C_SLAVE_WRITE_RECEIVED, &rx_data);
+ } else if (rx_status == I2C_SLAVE_RX_END &&
+ iproc_i2c->rx_start_rcvd) {
+ /* End of SMBUS Master write */
+ if (iproc_i2c->slave_rx_only)
+ i2c_slave_event(iproc_i2c->slave,
+ I2C_SLAVE_WRITE_RECEIVED,
+ &rx_data);
+
+ i2c_slave_event(iproc_i2c->slave, I2C
[PATCH v2 6/6] i2c: iproc: handle rx fifo full interrupt
Add code to handle IS_S_RX_FIFO_FULL_SHIFT interrupt to support
master write request with >= 64 bytes.
Iproc has a slave rx fifo size of 64 bytes.
Rx fifo full interrupt (IS_S_RX_FIFO_FULL_SHIFT) will be generated
when RX fifo becomes full. This can happen if master issues write
request of more than 64 bytes.
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 22e04055b447..cceaf69279a9 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -313,6 +313,8 @@ static void bcm_iproc_i2c_slave_init(
/* Enable interrupt register to indicate a valid byte in receive fifo */
val = BIT(IE_S_RX_EVENT_SHIFT);
+ /* Enable interrupt register to indicate Slave Rx FIFO Full */
+ val |= BIT(IE_S_RX_FIFO_FULL_SHIFT);
/* Enable interrupt register to indicate a Master read transaction */
val |= BIT(IE_S_RD_EVENT_SHIFT);
/* Enable interrupt register for the Slave BUSY command */
@@ -434,9 +436,15 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
*events
* Master-read : both IS_S_RX_EVENT_SHIFT and IS_S_RD_EVENT_SHIFT
*events or only IS_S_RD_EVENT_SHIFT
+*
+* iproc has a slave rx fifo size of 64 bytes. Rx fifo full interrupt
+* (IS_S_RX_FIFO_FULL_SHIFT) will be generated when RX fifo becomes
+* full. This can happen if Master issues write requests of more than
+* 64 bytes.
*/
if (status & BIT(IS_S_RX_EVENT_SHIFT) ||
- status & BIT(IS_S_RD_EVENT_SHIFT)) {
+ status & BIT(IS_S_RD_EVENT_SHIFT) ||
+ status & BIT(IS_S_RX_FIFO_FULL_SHIFT)) {
/* disable slave interrupts */
val = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
val &= ~iproc_i2c->slave_int_mask;
@@ -452,9 +460,14 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
/* schedule tasklet to read data later */
tasklet_schedule(&iproc_i2c->slave_rx_tasklet);
- /* clear only IS_S_RX_EVENT_SHIFT interrupt */
- iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET,
-BIT(IS_S_RX_EVENT_SHIFT));
+ /*
+* clear only IS_S_RX_EVENT_SHIFT and
+* IS_S_RX_FIFO_FULL_SHIFT interrupt.
+*/
+ val = BIT(IS_S_RX_EVENT_SHIFT);
+ if (status & BIT(IS_S_RX_FIFO_FULL_SHIFT))
+ val |= BIT(IS_S_RX_FIFO_FULL_SHIFT);
+ iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET, val);
}
if (status & BIT(IS_S_TX_UNDERRUN_SHIFT)) {
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v2 1/6] i2c: iproc: handle Master aborted error
Handle Master aborted error by flushing tx and rx fifo
and reinitializing the hw.
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index d8295b1c379d..834a98caeada 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -93,6 +93,7 @@
#define S_CMD_STATUS_MASK0x07
#define S_CMD_STATUS_SUCCESS 0x0
#define S_CMD_STATUS_TIMEOUT 0x5
+#define S_CMD_STATUS_MASTER_ABORT0x7
#define IE_OFFSET0x38
#define IE_M_RX_FIFO_FULL_SHIFT 31
@@ -311,9 +312,10 @@ static void bcm_iproc_i2c_check_slave_status(
return;
val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
- if (val == S_CMD_STATUS_TIMEOUT) {
- dev_err(iproc_i2c->device, "slave random stretch time
timeout\n");
-
+ if (val == S_CMD_STATUS_TIMEOUT || val == S_CMD_STATUS_MASTER_ABORT) {
+ dev_err(iproc_i2c->device, (val == S_CMD_STATUS_TIMEOUT) ?
+ "slave random stretch time timeout\n" :
+ "Master aborted read transaction\n");
/* re-initialize i2c for recovery */
bcm_iproc_i2c_enable_disable(iproc_i2c, false);
bcm_iproc_i2c_slave_init(iproc_i2c, true);
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v2 2/6] i2c: iproc: handle only slave interrupts which are enabled
Handle only slave interrupts which are enabled.
The IS_OFFSET register contains the interrupt status bits which will be
set regardless of the enabling of the corresponding interrupt condition.
One must therefore look at both IS_OFFSET and IE_OFFSET to determine
whether an interrupt condition is set and enabled.
Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
mode")
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 834a98caeada..b54f5130d246 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -507,12 +507,17 @@ static void bcm_iproc_i2c_process_m_event(struct
bcm_iproc_i2c_dev *iproc_i2c,
static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
{
struct bcm_iproc_i2c_dev *iproc_i2c = data;
- u32 status = iproc_i2c_rd_reg(iproc_i2c, IS_OFFSET);
+ u32 slave_status;
+ u32 status;
bool ret;
- u32 sl_status = status & ISR_MASK_SLAVE;
- if (sl_status) {
- ret = bcm_iproc_i2c_slave_isr(iproc_i2c, sl_status);
+ status = iproc_i2c_rd_reg(iproc_i2c, IS_OFFSET);
+ /* process only slave interrupt which are enabled */
+ slave_status = status & iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET) &
+ ISR_MASK_SLAVE;
+
+ if (slave_status) {
+ ret = bcm_iproc_i2c_slave_isr(iproc_i2c, slave_status);
if (ret)
return IRQ_HANDLED;
else
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v2 0/6] fix iproc driver to handle master read request
This series of patches adds the following, - Handle master abort error - Fix support for single/multi byte master read request with/without repeated start. - Handle rx fifo full interrupt - Fix typo Changes from V1: --Address review comments from Ray Jui, Remove fixes tag Rayagonda Kokatanur (6): i2c: iproc: handle Master aborted error i2c: iproc: handle only slave interrupts which are enabled i2c: iproc: update slave isr mask (ISR_MASK_SLAVE) i2c: iproc: fix typo in slave_isr function i2c: iproc: handle master read request i2c: iproc: handle rx fifo full interrupt drivers/i2c/busses/i2c-bcm-iproc.c | 254 +++-- 1 file changed, 200 insertions(+), 54 deletions(-) -- 2.17.1 smime.p7s Description: S/MIME Cryptographic Signature
[PATCH v2 3/6] i2c: iproc: update slave isr mask (ISR_MASK_SLAVE)
Update slave isr mask (ISR_MASK_SLAVE) to include remaining
two slave interrupts.
Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
mode")
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index b54f5130d246..cd687696bf0b 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -216,7 +216,8 @@ struct bcm_iproc_i2c_dev {
#define ISR_MASK_SLAVE (BIT(IS_S_START_BUSY_SHIFT)\
| BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT)\
- | BIT(IS_S_TX_UNDERRUN_SHIFT))
+ | BIT(IS_S_TX_UNDERRUN_SHIFT) | BIT(IS_S_RX_FIFO_FULL_SHIFT)\
+ | BIT(IS_S_RX_THLD_SHIFT))
static int bcm_iproc_i2c_reg_slave(struct i2c_client *slave);
static int bcm_iproc_i2c_unreg_slave(struct i2c_client *slave);
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v1 6/6] i2c: iproc: handle rx fifo full interrupt
Hi Dhanajay, On Fri, Oct 23, 2020 at 11:12 PM Ray Jui wrote: > > > > On 10/12/2020 3:03 PM, Dhananjay Phadke wrote: > > From: Rayagonda Kokatanur > > > > On Sun, 11 Oct 2020 23:52:54 +0530, Rayagonda Kokatanur wrote: > >> Add code to handle IS_S_RX_FIFO_FULL_SHIFT interrupt to support > >> master write request with >= 64 bytes. > >> > >> Iproc has a slave rx fifo size of 64 bytes. > >> Rx fifo full interrupt (IS_S_RX_FIFO_FULL_SHIFT) will be generated > >> when RX fifo becomes full. This can happen if master issues write > >> request of more than 64 bytes. > >> > > > > ARM cores run much faster than I2C bus, why would rx fifo go full when > > rx interrupt is enabled and bytes are read out by bus driver isr? > > Isn't fifo read pointer updated on these byte reads? > > Hi Rayagonda, > > Could you please reply on this question? For transactions > 64 bytes, do > we batch until RX FIFO is full before we read out the data? Sorry I missed this question. Yes with current design we are batching 64 bytes for translation > 64 bytes. Best regards, Rayagonda > > Thanks, > > Ray > > > Does controller stretch clock when rx fifo is full (e.g. kernel has > > crashed, bus driver isn't draining fifo)? > > smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v1 5/6] i2c: iproc: handle master read request
On Fri, Oct 23, 2020 at 10:56 PM Ray Jui wrote:
>
>
>
> On 10/13/2020 10:12 PM, Rayagonda Kokatanur wrote:
> >
> >
> > On Wed, Oct 14, 2020 at 8:50 AM Dhananjay Phadke
> > mailto:dpha...@linux.microsoft.com>> wrote:
> >
> > On Sun, 11 Oct 2020 23:52:53 +0530, Rayagonda Kokatanur wrote:
> > > --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> > > +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> > >
> > > - } else if (status & BIT(IS_S_RD_EVENT_SHIFT)) {
> > > - /* Start of SMBUS for Master Read */
> > > + I2C_SLAVE_WRITE_REQUESTED,
> > &rx_data);
> > > + iproc_i2c->rx_start_rcvd = true;
> > > + iproc_i2c->slave_read_complete = false;
> > > + } else if (rx_status == I2C_SLAVE_RX_DATA &&
> > > +iproc_i2c->rx_start_rcvd) {
> > > + /* Middle of SMBUS Master write */
> > > i2c_slave_event(iproc_i2c->slave,
> > > - I2C_SLAVE_READ_REQUESTED,
> > &value);
> > > - iproc_i2c_wr_reg(iproc_i2c, S_TX_OFFSET, value);
> > > + I2C_SLAVE_WRITE_RECEIVED,
> > &rx_data);
> > > + } else if (rx_status == I2C_SLAVE_RX_END &&
> > > +iproc_i2c->rx_start_rcvd) {
> > > + /* End of SMBUS Master write */
> > > + if (iproc_i2c->slave_rx_only)
> > > + i2c_slave_event(iproc_i2c->slave,
> > > +
> > I2C_SLAVE_WRITE_RECEIVED,
> > > + &rx_data);
> > > +
> > > + i2c_slave_event(iproc_i2c->slave,
> > I2C_SLAVE_STOP,
> > > + &rx_data);
> > > + } else if (rx_status == I2C_SLAVE_RX_FIFO_EMPTY) {
> > > + iproc_i2c->rx_start_rcvd = false;
> > > + iproc_i2c->slave_read_complete = true;
> > > + break;
> > > + }
> > >
> > > - val = BIT(S_CMD_START_BUSY_SHIFT);
> > > - iproc_i2c_wr_reg(iproc_i2c, S_CMD_OFFSET, val);
> > > + rx_bytes++;
> >
> > rx_bytes should be incremented only along with
> > I2C_SLAVE_WRITE_RECEIVED event?
> >
> >
> > It should be incremented in both I2C_SLAVE_WRITE_REQUESTED and
> > I2C_SLAVE_WRITE_RECEIVED cases because in both case it is reading valid
> > bytes from rx fifo.
> >
> >
> > >
> > > +static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev
> > *iproc_i2c,
> > > + u32 status)
> > > +{
> > > + u32 val;
> > > + u8 value;
> > > +
> > > + /*
> > > + * Slave events in case of master-write, master-write-read and,
> > > + * master-read
> > > + *
> > > + * Master-write : only IS_S_RX_EVENT_SHIFT event
> > > + * Master-write-read: both IS_S_RX_EVENT_SHIFT and
> > IS_S_RD_EVENT_SHIFT
> > > + *events
> > > + * Master-read : both IS_S_RX_EVENT_SHIFT and
> > IS_S_RD_EVENT_SHIFT
> > > + *events or only IS_S_RD_EVENT_SHIFT
> > > + */
> > > + if (status & BIT(IS_S_RX_EVENT_SHIFT) ||
> > > + status & BIT(IS_S_RD_EVENT_SHIFT)) {
> > > + /* disable slave interrupts */
> > > + val = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
> > > + val &= ~iproc_i2c->slave_int_mask;
> > > + iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, val);
> > > +
> > > + if (status & BIT(IS_S_RD_EVENT_SHIFT))
> > > + /* Master-write-read request */
> > > + iproc_i2c->slave_rx_only = false;
> > > + else
> > > + /* Master-write request only */
> > > + iproc_i2c->slave_
Re: [PATCH v1 4/6] i2c: iproc: fix typo in slave_isr function
On Fri, Oct 23, 2020 at 10:51 PM Ray Jui wrote:
>
>
>
> On 10/11/2020 11:22 AM, Rayagonda Kokatanur wrote:
> > Fix typo in bcm_iproc_i2c_slave_isr().
> >
> > Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for
> > slave mode")
>
> This is merely a fix of typo in code comment and there's no functional
> impact. Why do we need a Fixes tag on this (which indicates the fix
> needs to be backported to LTS kernels)?
Thank you Ray.
I will remove Fixes tag and push patch v2.
Best regards,
Rayagonda
>
> > Signed-off-by: Rayagonda Kokatanur
> > ---
> > drivers/i2c/busses/i2c-bcm-iproc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
> > b/drivers/i2c/busses/i2c-bcm-iproc.c
> > index cd687696bf0b..7a235f9f5884 100644
> > --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> > +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> > @@ -382,7 +382,7 @@ static bool bcm_iproc_i2c_slave_isr(struct
> > bcm_iproc_i2c_dev *iproc_i2c,
> > if (status & BIT(IS_S_START_BUSY_SHIFT)) {
> > i2c_slave_event(iproc_i2c->slave, I2C_SLAVE_STOP, &value);
> > /*
> > - * Enable interrupt for TX FIFO becomes empty and
> > + * Disable interrupt for TX FIFO becomes empty and
> >* less than PKT_LENGTH bytes were output on the SMBUS
> >*/
> > val = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
> >
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v1 5/6] i2c: iproc: handle master read request
On Wed, Oct 14, 2020 at 8:50 AM Dhananjay Phadke
wrote:
>
> On Sun, 11 Oct 2020 23:52:53 +0530, Rayagonda Kokatanur wrote:
> > --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> > +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> >
> > - } else if (status & BIT(IS_S_RD_EVENT_SHIFT)) {
> > - /* Start of SMBUS for Master Read */
> > + I2C_SLAVE_WRITE_REQUESTED, &rx_data);
> > + iproc_i2c->rx_start_rcvd = true;
> > + iproc_i2c->slave_read_complete = false;
> > + } else if (rx_status == I2C_SLAVE_RX_DATA &&
> > +iproc_i2c->rx_start_rcvd) {
> > + /* Middle of SMBUS Master write */
> > i2c_slave_event(iproc_i2c->slave,
> > - I2C_SLAVE_READ_REQUESTED, &value);
> > - iproc_i2c_wr_reg(iproc_i2c, S_TX_OFFSET, value);
> > + I2C_SLAVE_WRITE_RECEIVED, &rx_data);
> > + } else if (rx_status == I2C_SLAVE_RX_END &&
> > +iproc_i2c->rx_start_rcvd) {
> > + /* End of SMBUS Master write */
> > + if (iproc_i2c->slave_rx_only)
> > + i2c_slave_event(iproc_i2c->slave,
> > + I2C_SLAVE_WRITE_RECEIVED,
> > + &rx_data);
> > +
> > + i2c_slave_event(iproc_i2c->slave, I2C_SLAVE_STOP,
> > + &rx_data);
> > + } else if (rx_status == I2C_SLAVE_RX_FIFO_EMPTY) {
> > + iproc_i2c->rx_start_rcvd = false;
> > + iproc_i2c->slave_read_complete = true;
> > + break;
> > + }
> >
> > - val = BIT(S_CMD_START_BUSY_SHIFT);
> > - iproc_i2c_wr_reg(iproc_i2c, S_CMD_OFFSET, val);
> > + rx_bytes++;
>
> rx_bytes should be incremented only along with I2C_SLAVE_WRITE_RECEIVED event?
It should be incremented in both I2C_SLAVE_WRITE_REQUESTED and
I2C_SLAVE_WRITE_RECEIVED cases because in both cases it is reading
valid bytes from rx fifo.
>
> >
> > +static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
> > + u32 status)
> > +{
> > + u32 val;
> > + u8 value;
> > +
> > + /*
> > + * Slave events in case of master-write, master-write-read and,
> > + * master-read
> > + *
> > + * Master-write : only IS_S_RX_EVENT_SHIFT event
> > + * Master-write-read: both IS_S_RX_EVENT_SHIFT and IS_S_RD_EVENT_SHIFT
> > + *events
> > + * Master-read : both IS_S_RX_EVENT_SHIFT and IS_S_RD_EVENT_SHIFT
> > + *events or only IS_S_RD_EVENT_SHIFT
> > + */
> > + if (status & BIT(IS_S_RX_EVENT_SHIFT) ||
> > + status & BIT(IS_S_RD_EVENT_SHIFT)) {
> > + /* disable slave interrupts */
> > + val = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
> > + val &= ~iproc_i2c->slave_int_mask;
> > + iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, val);
> > +
> > + if (status & BIT(IS_S_RD_EVENT_SHIFT))
> > + /* Master-write-read request */
> > + iproc_i2c->slave_rx_only = false;
> > + else
> > + /* Master-write request only */
> > + iproc_i2c->slave_rx_only = true;
> > +
> > + /* schedule tasklet to read data later */
> > + tasklet_schedule(&iproc_i2c->slave_rx_tasklet);
> > +
> > + /* clear only IS_S_RX_EVENT_SHIFT interrupt */
> > + iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET,
> > + BIT(IS_S_RX_EVENT_SHIFT));
> >
>
> Both tasklet and isr are writing to status (IS_OFFSET) reg.
>
> The tasklet seems to be batching up rx fifo reads because of time-sensitive
> Master-write-read transaction? Linux I2C framework is byte interface anyway.
> Can the need to batch reads be avoided by setting slave rx threshold for
> interrupt (S_FIFO_RX_THLD) to 1-byte?
To process more data with a single interrupt we are batching up rx fifo reads.
This will reduce the number of interrupts.
Also to avoid tasklet running more time (20us) we have a threshold of
10 bytes for batching read.
This is a better/optimised approach than reading single byte data per interrupt.
>
> Also, wouldn't tasklets be susceptible to other interrupts? If fifo reads
> have to be batched up, can it be changed to threaded irq?
tasklets have higher priority than threaded irq, since i2c is time
sensitive so using a tasklet is preferred over threaded irq.
Best regards,
Rayagonda
>
>
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v1 5/6] i2c: iproc: handle master read request
Handle single or multi byte master read request with or without
repeated start.
Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
mode")
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 215 +++--
1 file changed, 170 insertions(+), 45 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 7a235f9f5884..22e04055b447 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -160,6 +160,11 @@
#define IE_S_ALL_INTERRUPT_SHIFT 21
#define IE_S_ALL_INTERRUPT_MASK 0x3f
+/*
+ * It takes ~18us to reading 10bytes of data, hence to keep tasklet
+ * running for less time, max slave read per tasklet is set to 10 bytes.
+ */
+#define MAX_SLAVE_RX_PER_INT 10
enum i2c_slave_read_status {
I2C_SLAVE_RX_FIFO_EMPTY = 0,
@@ -206,8 +211,18 @@ struct bcm_iproc_i2c_dev {
/* bytes that have been read */
unsigned int rx_bytes;
unsigned int thld_bytes;
+
+ bool slave_rx_only;
+ bool rx_start_rcvd;
+ bool slave_read_complete;
+ u32 tx_underrun;
+ u32 slave_int_mask;
+ struct tasklet_struct slave_rx_tasklet;
};
+/* tasklet to process slave rx data */
+static void slave_rx_tasklet_fn(unsigned long);
+
/*
* Can be expanded in the future if more interrupt status bits are utilized
*/
@@ -261,6 +276,7 @@ static void bcm_iproc_i2c_slave_init(
{
u32 val;
+ iproc_i2c->tx_underrun = 0;
if (need_reset) {
/* put controller in reset */
val = iproc_i2c_rd_reg(iproc_i2c, CFG_OFFSET);
@@ -297,8 +313,11 @@ static void bcm_iproc_i2c_slave_init(
/* Enable interrupt register to indicate a valid byte in receive fifo */
val = BIT(IE_S_RX_EVENT_SHIFT);
+ /* Enable interrupt register to indicate a Master read transaction */
+ val |= BIT(IE_S_RD_EVENT_SHIFT);
/* Enable interrupt register for the Slave BUSY command */
val |= BIT(IE_S_START_BUSY_SHIFT);
+ iproc_i2c->slave_int_mask = val;
iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, val);
}
@@ -324,76 +343,176 @@ static void bcm_iproc_i2c_check_slave_status(
}
}
-static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
- u32 status)
+static void bcm_iproc_i2c_slave_read(struct bcm_iproc_i2c_dev *iproc_i2c)
{
+ u8 rx_data, rx_status;
+ u32 rx_bytes = 0;
u32 val;
- u8 value, rx_status;
- /* Slave RX byte receive */
- if (status & BIT(IS_S_RX_EVENT_SHIFT)) {
+ while (rx_bytes < MAX_SLAVE_RX_PER_INT) {
val = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
rx_status = (val >> S_RX_STATUS_SHIFT) & S_RX_STATUS_MASK;
- if (rx_status == I2C_SLAVE_RX_START) {
- /* Start of SMBUS for Master write */
- i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_WRITE_REQUESTED, &value);
+ rx_data = ((val >> S_RX_DATA_SHIFT) & S_RX_DATA_MASK);
- val = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
- value = (u8)((val >> S_RX_DATA_SHIFT) & S_RX_DATA_MASK);
+ if (rx_status == I2C_SLAVE_RX_START) {
+ /* Start of SMBUS Master write */
i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_WRITE_RECEIVED, &value);
- } else if (status & BIT(IS_S_RD_EVENT_SHIFT)) {
- /* Start of SMBUS for Master Read */
+ I2C_SLAVE_WRITE_REQUESTED, &rx_data);
+ iproc_i2c->rx_start_rcvd = true;
+ iproc_i2c->slave_read_complete = false;
+ } else if (rx_status == I2C_SLAVE_RX_DATA &&
+ iproc_i2c->rx_start_rcvd) {
+ /* Middle of SMBUS Master write */
i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_READ_REQUESTED, &value);
- iproc_i2c_wr_reg(iproc_i2c, S_TX_OFFSET, value);
+ I2C_SLAVE_WRITE_RECEIVED, &rx_data);
+ } else if (rx_status == I2C_SLAVE_RX_END &&
+ iproc_i2c->rx_start_rcvd) {
+ /* End of SMBUS Master write */
+ if (iproc_i2c->slave_rx_only)
+ i2c_slave_event(iproc_i2c->slave,
+ I2C_SLAVE_WRITE_RECEIVED,
+ &rx_data);
+
+ i2c_slave_event(iproc_i2c->slave, I2C
[PATCH v1 1/6] i2c: iproc: handle Master aborted error
Handle Master aborted error by flushing tx and rx fifo
and reinitializing the hw.
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index d8295b1c379d..834a98caeada 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -93,6 +93,7 @@
#define S_CMD_STATUS_MASK0x07
#define S_CMD_STATUS_SUCCESS 0x0
#define S_CMD_STATUS_TIMEOUT 0x5
+#define S_CMD_STATUS_MASTER_ABORT0x7
#define IE_OFFSET0x38
#define IE_M_RX_FIFO_FULL_SHIFT 31
@@ -311,9 +312,10 @@ static void bcm_iproc_i2c_check_slave_status(
return;
val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
- if (val == S_CMD_STATUS_TIMEOUT) {
- dev_err(iproc_i2c->device, "slave random stretch time
timeout\n");
-
+ if (val == S_CMD_STATUS_TIMEOUT || val == S_CMD_STATUS_MASTER_ABORT) {
+ dev_err(iproc_i2c->device, (val == S_CMD_STATUS_TIMEOUT) ?
+ "slave random stretch time timeout\n" :
+ "Master aborted read transaction\n");
/* re-initialize i2c for recovery */
bcm_iproc_i2c_enable_disable(iproc_i2c, false);
bcm_iproc_i2c_slave_init(iproc_i2c, true);
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v1 3/6] i2c: iproc: update slave isr mask (ISR_MASK_SLAVE)
Update slave isr mask (ISR_MASK_SLAVE) to include remaining
two slave interrupts.
Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
mode")
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index b54f5130d246..cd687696bf0b 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -216,7 +216,8 @@ struct bcm_iproc_i2c_dev {
#define ISR_MASK_SLAVE (BIT(IS_S_START_BUSY_SHIFT)\
| BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT)\
- | BIT(IS_S_TX_UNDERRUN_SHIFT))
+ | BIT(IS_S_TX_UNDERRUN_SHIFT) | BIT(IS_S_RX_FIFO_FULL_SHIFT)\
+ | BIT(IS_S_RX_THLD_SHIFT))
static int bcm_iproc_i2c_reg_slave(struct i2c_client *slave);
static int bcm_iproc_i2c_unreg_slave(struct i2c_client *slave);
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v1 4/6] i2c: iproc: fix typo in slave_isr function
Fix typo in bcm_iproc_i2c_slave_isr().
Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
mode")
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index cd687696bf0b..7a235f9f5884 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -382,7 +382,7 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
if (status & BIT(IS_S_START_BUSY_SHIFT)) {
i2c_slave_event(iproc_i2c->slave, I2C_SLAVE_STOP, &value);
/*
-* Enable interrupt for TX FIFO becomes empty and
+* Disable interrupt for TX FIFO becomes empty and
* less than PKT_LENGTH bytes were output on the SMBUS
*/
val = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v1 6/6] i2c: iproc: handle rx fifo full interrupt
Add code to handle IS_S_RX_FIFO_FULL_SHIFT interrupt to support
master write request with >= 64 bytes.
Iproc has a slave rx fifo size of 64 bytes.
Rx fifo full interrupt (IS_S_RX_FIFO_FULL_SHIFT) will be generated
when RX fifo becomes full. This can happen if master issues write
request of more than 64 bytes.
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 22e04055b447..cceaf69279a9 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -313,6 +313,8 @@ static void bcm_iproc_i2c_slave_init(
/* Enable interrupt register to indicate a valid byte in receive fifo */
val = BIT(IE_S_RX_EVENT_SHIFT);
+ /* Enable interrupt register to indicate Slave Rx FIFO Full */
+ val |= BIT(IE_S_RX_FIFO_FULL_SHIFT);
/* Enable interrupt register to indicate a Master read transaction */
val |= BIT(IE_S_RD_EVENT_SHIFT);
/* Enable interrupt register for the Slave BUSY command */
@@ -434,9 +436,15 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
*events
* Master-read : both IS_S_RX_EVENT_SHIFT and IS_S_RD_EVENT_SHIFT
*events or only IS_S_RD_EVENT_SHIFT
+*
+* iproc has a slave rx fifo size of 64 bytes. Rx fifo full interrupt
+* (IS_S_RX_FIFO_FULL_SHIFT) will be generated when RX fifo becomes
+* full. This can happen if Master issues write requests of more than
+* 64 bytes.
*/
if (status & BIT(IS_S_RX_EVENT_SHIFT) ||
- status & BIT(IS_S_RD_EVENT_SHIFT)) {
+ status & BIT(IS_S_RD_EVENT_SHIFT) ||
+ status & BIT(IS_S_RX_FIFO_FULL_SHIFT)) {
/* disable slave interrupts */
val = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
val &= ~iproc_i2c->slave_int_mask;
@@ -452,9 +460,14 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
/* schedule tasklet to read data later */
tasklet_schedule(&iproc_i2c->slave_rx_tasklet);
- /* clear only IS_S_RX_EVENT_SHIFT interrupt */
- iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET,
-BIT(IS_S_RX_EVENT_SHIFT));
+ /*
+* clear only IS_S_RX_EVENT_SHIFT and
+* IS_S_RX_FIFO_FULL_SHIFT interrupt.
+*/
+ val = BIT(IS_S_RX_EVENT_SHIFT);
+ if (status & BIT(IS_S_RX_FIFO_FULL_SHIFT))
+ val |= BIT(IS_S_RX_FIFO_FULL_SHIFT);
+ iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET, val);
}
if (status & BIT(IS_S_TX_UNDERRUN_SHIFT)) {
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v1 2/6] i2c: iproc: handle only slave interrupts which are enabled
Handle only slave interrupts which are enabled.
The IS_OFFSET register contains the interrupt status bits which will be
set regardless of the enabling of the corresponding interrupt condition.
One must therefore look at both IS_OFFSET and IE_OFFSET to determine
whether an interrupt condition is set and enabled.
Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
mode")
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 834a98caeada..b54f5130d246 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -507,12 +507,17 @@ static void bcm_iproc_i2c_process_m_event(struct
bcm_iproc_i2c_dev *iproc_i2c,
static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
{
struct bcm_iproc_i2c_dev *iproc_i2c = data;
- u32 status = iproc_i2c_rd_reg(iproc_i2c, IS_OFFSET);
+ u32 slave_status;
+ u32 status;
bool ret;
- u32 sl_status = status & ISR_MASK_SLAVE;
- if (sl_status) {
- ret = bcm_iproc_i2c_slave_isr(iproc_i2c, sl_status);
+ status = iproc_i2c_rd_reg(iproc_i2c, IS_OFFSET);
+ /* process only slave interrupt which are enabled */
+ slave_status = status & iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET) &
+ ISR_MASK_SLAVE;
+
+ if (slave_status) {
+ ret = bcm_iproc_i2c_slave_isr(iproc_i2c, slave_status);
if (ret)
return IRQ_HANDLED;
else
--
2.17.1
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH v1 0/6] fix iproc driver to handle master read request
This series of patches adds the following, - Handle master abort error - Fix support for single/multi byte master read request with/without repeated start. - Handle rx fifo full interrupt - Fix typo Rayagonda Kokatanur (6): i2c: iproc: handle Master aborted error i2c: iproc: handle only slave interrupts which are enabled i2c: iproc: update slave isr mask (ISR_MASK_SLAVE) i2c: iproc: fix typo in slave_isr function i2c: iproc: handle master read request i2c: iproc: handle rx fifo full interrupt drivers/i2c/busses/i2c-bcm-iproc.c | 254 +++-- 1 file changed, 200 insertions(+), 54 deletions(-) -- 2.17.1 smime.p7s Description: S/MIME Cryptographic Signature
[PATCH v1 1/1] firmware: tee_bnxt: reduce shared mem size to 4K
Reduce the DMA shared memory for OP-TEE and Linux to 4K. Signed-off-by: Rayagonda Kokatanur --- drivers/firmware/broadcom/tee_bnxt_fw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/broadcom/tee_bnxt_fw.c b/drivers/firmware/broadcom/tee_bnxt_fw.c index ed10da5313e8..3da6a01aff9b 100644 --- a/drivers/firmware/broadcom/tee_bnxt_fw.c +++ b/drivers/firmware/broadcom/tee_bnxt_fw.c @@ -12,7 +12,7 @@ #include -#define MAX_SHM_MEM_SZ SZ_4M +#define MAX_SHM_MEM_SZ SZ_4K #define MAX_TEE_PARAM_ARRY_MEMB4 -- 2.17.1 smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] i2c: iproc: fix race between client unreg and isr
On Sat, Jul 25, 2020 at 3:48 PM Wolfram Sang wrote: > > > > I think the following sequence needs to be implemented to make this > > safe, i.e., after 'synchronize_irq', no further slave interrupt will be > > fired. > > > > In 'bcm_iproc_i2c_unreg_slave': > > > > 1. Set an atomic variable 'unreg_slave' (I'm bad in names so please come > > up with a better name than this) > > > > 2. Disable all slave interrupts > > > > 3. synchronize_irq > > > > 4. Set slave to NULL > > > > 5. Erase slave addresses > > What about this in unreg_slave? > > 1. disable_irq() > This includes synchronize_irq() and avoids the race. Because irq > will be masked at interrupt controller level, interrupts coming > in at the I2C IP core level should still be pending once we > reenable the irq. > > 2. disable all slave interrupts > > 3. enable_irq() > > 4. clean up the rest (pointer, address) > > Or am I overlooking something? This sequence will take care of all cases. @Dhananjay Phadke is it possible to verify this from your side once. Best regards, Raaygonda
Re: [PATCH v2 2/2] i2c: iproc: add slave pec support
On Fri, Jul 24, 2020 at 1:50 AM Wolfram Sang wrote: > > > > + /* Enable partial slave HW PEC support if requested by the client */ > > + iproc_i2c->en_s_pec = !!(slave->flags & I2C_CLIENT_PEC); > > + if (iproc_i2c->en_s_pec) > > + dev_info(iproc_i2c->device, "Enable PEC\n"); > > Where do you set the I2C_CLIENT_PEC flag for the slave? Is your backend > code publicly available? I2C_CLIENT_PEC should be set by backend before calling i2c_slave_register() ie client->flags |= I2C_CLIENT_PEC; ret = i2c_slave_register(client, i2c_slave_eeprom_slave_cb); -- -- -- My backend code is not yet publicly available. > > I may need a second thought here, but I am not sure I2C_CLIENT_PEC is > the right way to enable PEC. Isn't it actually depending on the backend > if PEC is needed? I.e. is the backend an I2C device or an SMBus device? > Yes, it depends on the backend. If backend is SMBUS device and supports PEC then it should set client->flags |= I2C_CLIENT_PEC, before calling i2c_slave_register(), so that the slave bus driver will enable PEC in device. Best regards, Rayagonda
Re: [PATCH v2 1/2] i2c: add PEC error event
On Fri, Jul 24, 2020 at 1:46 AM Wolfram Sang wrote:
>
> On Fri, Jul 17, 2020 at 02:31:54PM +0530, Rayagonda Kokatanur wrote:
> > Add new event I2C_SLAVE_PEC_ERR to list of slave events.
> > This event will be used by slave bus driver to indicate
> > PEC error to slave client or backend driver.
> >
> > Signed-off-by: Rayagonda Kokatanur
>
> Definately needs documentation in Documentation/i2c/slave-interface.rst.
Okay, I will update the doc.
>
> What is a backend supposed to do? Does 'value' have a meaning?
According to SMBUS spec, during slave receive transfer, it is
encouraged to issue NACK if the received PEC is not correct.
Also PEC errors discovered above the data link layer may also be
indicated with a NACK if the device is fast enough to discover and
indicate the error when the NACK is due.
If a device doesn't support issuing NACK and issuing NACK above the
data link layer is not possible because the device is not fast then in
that case we can send error code (I2C_SLAVE_PEC_ERR ) to the backend
driver. Backend drivers which support PEC should check for this error
code and take action such as discarding the data.
Best regards,
Rayagonda
>
> > ---
> > include/linux/i2c.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index b8b8963f8bb9..e04acd04eb48 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -370,6 +370,7 @@ enum i2c_slave_event {
> > I2C_SLAVE_READ_PROCESSED,
> > I2C_SLAVE_WRITE_RECEIVED,
> > I2C_SLAVE_STOP,
> > + I2C_SLAVE_PEC_ERR,
> > };
> >
> > int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
> > --
> > 2.17.1
> >
Re: [PATCH] i2c: iproc: fix race between client unreg and isr
On Sun, Jul 19, 2020 at 5:10 AM Dhananjay Phadke
wrote:
>
> When i2c client unregisters, synchronize irq before setting
> iproc_i2c->slave to NULL.
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 0318
>
> [ 371.020421] pc : bcm_iproc_i2c_isr+0x530/0x11f0
> [ 371.025098] lr : __handle_irq_event_percpu+0x6c/0x170
> [ 371.030309] sp : 800010003e40
> [ 371.033727] x29: 800010003e40 x28: 0060
> [ 371.039206] x27: 800010ca9de0 x26: 800010f895df
> [ 371.044686] x25: 800010f1 x24: 0008f7ff3600
> [ 371.050165] x23: 0003 x22: 0160
> [ 371.055645] x21: 800010f1 x20: 0160
> [ 371.061124] x19: 0008f726f080 x18:
> [ 371.066603] x17: x16:
> [ 371.072082] x15: x14:
> [ 371.077561] x13: x12: 0001
> [ 371.083040] x11: x10: 0040
> [ 371.088519] x9 : 800010f317c8 x8 : 800010f317c0
> [ 371.093999] x7 : 0008f805b3b0 x6 :
> [ 371.099478] x5 : 0008f7ff36a4 x4 : 8008ee43d000
> [ 371.104957] x3 : x2 : 8000107d64c0
> [ 371.110436] x1 : c0af x0 :
>
> [ 371.115916] Call trace:
> [ 371.118439] bcm_iproc_i2c_isr+0x530/0x11f0
> [ 371.122754] __handle_irq_event_percpu+0x6c/0x170
> [ 371.127606] handle_irq_event_percpu+0x34/0x88
> [ 371.132189] handle_irq_event+0x40/0x120
> [ 371.136234] handle_fasteoi_irq+0xcc/0x1a0
> [ 371.140459] generic_handle_irq+0x24/0x38
> [ 371.144594] __handle_domain_irq+0x60/0xb8
> [ 371.148820] gic_handle_irq+0xc0/0x158
> [ 371.152687] el1_irq+0xb8/0x140
> [ 371.155927] arch_cpu_idle+0x10/0x18
> [ 371.159615] do_idle+0x204/0x290
> [ 371.162943] cpu_startup_entry+0x24/0x60
> [ 371.166990] rest_init+0xb0/0xbc
> [ 371.170322] arch_call_rest_init+0xc/0x14
> [ 371.174458] start_kernel+0x404/0x430
>
> Fixes: c245d94ed106 ("i2c: iproc: Add multi byte read-write support for slave
> mode")
> Signed-off-by: Dhananjay Phadke
> ---
> drivers/i2c/busses/i2c-bcm-iproc.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
> b/drivers/i2c/busses/i2c-bcm-iproc.c
> index b58224b7b..37d2a79e7 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -1074,14 +1074,15 @@ static int bcm_iproc_i2c_unreg_slave(struct
> i2c_client *slave)
> if (!iproc_i2c->slave)
> return -EINVAL;
>
> - iproc_i2c->slave = NULL;
> -
> /* disable all slave interrupts */
> tmp = iproc_i2c_rd_reg(iproc_i2c, IE_OFFSET);
> tmp &= ~(IE_S_ALL_INTERRUPT_MASK <<
> IE_S_ALL_INTERRUPT_SHIFT);
> iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, tmp);
>
> + synchronize_irq(iproc_i2c->irq);
> + iproc_i2c->slave = NULL;
> +
> /* Erase the slave address programmed */
> tmp = iproc_i2c_rd_reg(iproc_i2c, S_CFG_SMBUS_ADDR_OFFSET);
> tmp &= ~BIT(S_CFG_EN_NIC_SMB_ADDR3_SHIFT);
Looks good to me. Thank you for the patch.
Reviewed-by: Rayagonda Kokatanur
Regards,
Rayagonda
[PATCH v2 2/2] i2c: iproc: add slave pec support
Iproc supports PEC computation and checking in both Master
and Slave mode.
This patch adds support for PEC in slave mode.
As per hw spec, PEC ERROR status bits are [29:28] in S_RX_OFFSET
register, hence this patch corrects the S_RX_PEC_ERR_SHIFT.
Signed-off-by: Rayagonda Kokatanur
---
Changes from v1:
-Address review comments from Andy Shevchenko
Update commit message,
Rewrite bcm_iproc_smbus_check_slave_pec() to remove local
variable ret and type casting,
Use positive condition.
drivers/i2c/busses/i2c-bcm-iproc.c | 49 +++---
1 file changed, 45 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 8a3c98866fb7..cfa7b044209e 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -93,6 +93,7 @@
#define S_CMD_STATUS_MASK0x07
#define S_CMD_STATUS_SUCCESS 0x0
#define S_CMD_STATUS_TIMEOUT 0x5
+#define S_CMD_PEC_SHIFT 8
#define IE_OFFSET0x38
#define IE_M_RX_FIFO_FULL_SHIFT 31
@@ -138,7 +139,9 @@
#define S_RX_OFFSET 0x4c
#define S_RX_STATUS_SHIFT30
#define S_RX_STATUS_MASK 0x03
-#define S_RX_PEC_ERR_SHIFT 29
+#define S_RX_PEC_ERR_SHIFT 28
+#define S_RX_PEC_ERR_MASK0x3
+#define S_RX_PEC_ERR 0x1
#define S_RX_DATA_SHIFT 0
#define S_RX_DATA_MASK 0xff
@@ -205,6 +208,8 @@ struct bcm_iproc_i2c_dev {
/* bytes that have been read */
unsigned int rx_bytes;
unsigned int thld_bytes;
+
+ bool en_s_pec;
};
/*
@@ -321,6 +326,23 @@ static void bcm_iproc_i2c_check_slave_status(
}
}
+static int bcm_iproc_smbus_check_slave_pec(struct bcm_iproc_i2c_dev *iproc_i2c,
+ u32 val)
+{
+ u8 err_status;
+
+ if (!iproc_i2c->en_s_pec)
+ return 0;
+
+ err_status = (val >> S_RX_PEC_ERR_SHIFT) & S_RX_PEC_ERR_MASK;
+ if (err_status == S_RX_PEC_ERR) {
+ dev_err(iproc_i2c->device, "Slave PEC error\n");
+ return -EBADMSG;
+ }
+
+ return 0;
+}
+
static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
u32 status)
{
@@ -347,6 +369,8 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
iproc_i2c_wr_reg(iproc_i2c, S_TX_OFFSET, value);
val = BIT(S_CMD_START_BUSY_SHIFT);
+ if (iproc_i2c->en_s_pec)
+ val |= BIT(S_CMD_PEC_SHIFT);
iproc_i2c_wr_reg(iproc_i2c, S_CMD_OFFSET, val);
/*
@@ -361,9 +385,19 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
value = (u8)((val >> S_RX_DATA_SHIFT) & S_RX_DATA_MASK);
i2c_slave_event(iproc_i2c->slave,
I2C_SLAVE_WRITE_RECEIVED, &value);
- if (rx_status == I2C_SLAVE_RX_END)
- i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_STOP, &value);
+ if (rx_status == I2C_SLAVE_RX_END) {
+ int ret;
+
+ ret = bcm_iproc_smbus_check_slave_pec(iproc_i2c,
+ val);
+ if (ret)
+ i2c_slave_event(iproc_i2c->slave,
+ I2C_SLAVE_PEC_ERR,
+ &value);
+ else
+ i2c_slave_event(iproc_i2c->slave,
+ I2C_SLAVE_STOP, &value);
+ }
}
} else if (status & BIT(IS_S_TX_UNDERRUN_SHIFT)) {
/* Master read other than start */
@@ -372,6 +406,8 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
iproc_i2c_wr_reg(iproc_i2c, S_TX_OFFSET, value);
val = BIT(S_CMD_START_BUSY_SHIFT);
+ if (iproc_i2c->en_s_pec)
+ val |= BIT(S_CMD_PEC_SHIFT);
iproc_i2c_wr_reg(iproc_i2c, S_CMD_OFFSET, val);
}
@@ -1065,6 +1101,11 @@ static int bcm_iproc_i2c_reg_slave(struct i2c_client
*slave)
if (slave->flags & I2C_CLIENT_TEN)
return -EAFNOSUPPORT;
+ /* Enable partial slave HW PEC support if requested by the client */
+ iproc_i2c->en_s_pec = !!(slave->flags & I2C_CLIENT_PEC);
+ if (iproc_i2c->en_s_pec)
+
[PATCH v2 1/2] i2c: add PEC error event
Add new event I2C_SLAVE_PEC_ERR to list of slave events.
This event will be used by slave bus driver to indicate
PEC error to slave client or backend driver.
Signed-off-by: Rayagonda Kokatanur
---
include/linux/i2c.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b8b8963f8bb9..e04acd04eb48 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -370,6 +370,7 @@ enum i2c_slave_event {
I2C_SLAVE_READ_PROCESSED,
I2C_SLAVE_WRITE_RECEIVED,
I2C_SLAVE_STOP,
+ I2C_SLAVE_PEC_ERR,
};
int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
--
2.17.1
[PATCH v2 0/2] add PEC support on slave side
This patch set adds support for PEC on Slave side. Changes from v1: -Address review comments from Andy Shevchenko Update commit message, Rewrite bcm_iproc_smbus_check_slave_pec() to remove local variable ret and type casting, Use positive condition. Rayagonda Kokatanur (2): i2c: add PEC error event i2c: iproc: add slave pec support drivers/i2c/busses/i2c-bcm-iproc.c | 49 +++--- include/linux/i2c.h| 1 + 2 files changed, 46 insertions(+), 4 deletions(-) -- 2.17.1
Re: [PATCH V1 2/2] i2c: iproc: add slave pec support
Hi Andy,
On Thu, Jul 16, 2020 at 3:44 PM Andy Shevchenko
wrote:
>
> On Thu, Jul 16, 2020 at 11:14 AM Rayagonda Kokatanur
> wrote:
> >
> > Iproc supports PEC computation and checking in both Master
> > and Slave mode.
> >
> > This patch adds support for PEC in slave mode.
>
> ...
>
> > -#define S_RX_PEC_ERR_SHIFT 29
> > +#define S_RX_PEC_ERR_SHIFT 28
> > +#define S_RX_PEC_ERR_MASK0x3
> > +#define S_RX_PEC_ERR 0x1
>
> This needs to be explained in the commit message, in particular why
> this change makes no regression.
I didn't get what do you mean by "no regression", please elaborate.
>
> ...
>
> > +static int bcm_iproc_smbus_check_slave_pec(struct bcm_iproc_i2c_dev
> > *iproc_i2c,
> > + u32 val)
> > +{
> > + u8 err_status;
>
> > + int ret = 0;
>
> Completely redundant variable.
>
> > + if (!iproc_i2c->en_s_pec)
> > + return ret;
>
> return 0;
>
> > + err_status = (u8)((val >> S_RX_PEC_ERR_SHIFT) & S_RX_PEC_ERR_MASK);
>
> Why casting?
>
> > + if (err_status == S_RX_PEC_ERR) {
> > + dev_err(iproc_i2c->device, "Slave PEC error\n");
>
> > + ret = -EBADMSG;
>
> return ...
>
> > + }
> > +
> > + return ret;
>
> return 0;
>
> > +}
>
> ...
>
> > + if (rx_status == I2C_SLAVE_RX_END) {
> > + int ret;
> > +
> > + ret =
> > bcm_iproc_smbus_check_slave_pec(iproc_i2c,
> > + val);
>
> One line looks better.
Yes, but to have 80 char per line, I have to do this.
>
> > + if (!ret)
>
> Why not positive conditional?
Thank you for your review.
Will fix all above.
Best regards,
Rayagonda
>
> > + i2c_slave_event(iproc_i2c->slave,
> > + I2C_SLAVE_STOP,
> > &value);
> > + else
> > + i2c_slave_event(iproc_i2c->slave,
> > + I2C_SLAVE_PEC_ERR,
> > + &value);
> > + }
>
> --
> With Best Regards,
> Andy Shevchenko
[PATCH V1 1/2] i2c: add PEC error event
Add new event I2C_SLAVE_PEC_ERR to list of slave events.
This event will be used by slave bus driver to indicate
PEC error to slave client or backend driver.
Signed-off-by: Rayagonda Kokatanur
---
include/linux/i2c.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b8b8963f8bb9..e04acd04eb48 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -370,6 +370,7 @@ enum i2c_slave_event {
I2C_SLAVE_READ_PROCESSED,
I2C_SLAVE_WRITE_RECEIVED,
I2C_SLAVE_STOP,
+ I2C_SLAVE_PEC_ERR,
};
int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
--
2.17.1
[PATCH V1 2/2] i2c: iproc: add slave pec support
Iproc supports PEC computation and checking in both Master
and Slave mode.
This patch adds support for PEC in slave mode.
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 50 +++---
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 8a3c98866fb7..51c8b165bb5e 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -93,6 +93,7 @@
#define S_CMD_STATUS_MASK0x07
#define S_CMD_STATUS_SUCCESS 0x0
#define S_CMD_STATUS_TIMEOUT 0x5
+#define S_CMD_PEC_SHIFT 8
#define IE_OFFSET0x38
#define IE_M_RX_FIFO_FULL_SHIFT 31
@@ -138,7 +139,9 @@
#define S_RX_OFFSET 0x4c
#define S_RX_STATUS_SHIFT30
#define S_RX_STATUS_MASK 0x03
-#define S_RX_PEC_ERR_SHIFT 29
+#define S_RX_PEC_ERR_SHIFT 28
+#define S_RX_PEC_ERR_MASK0x3
+#define S_RX_PEC_ERR 0x1
#define S_RX_DATA_SHIFT 0
#define S_RX_DATA_MASK 0xff
@@ -205,6 +208,8 @@ struct bcm_iproc_i2c_dev {
/* bytes that have been read */
unsigned int rx_bytes;
unsigned int thld_bytes;
+
+ bool en_s_pec;
};
/*
@@ -321,6 +326,24 @@ static void bcm_iproc_i2c_check_slave_status(
}
}
+static int bcm_iproc_smbus_check_slave_pec(struct bcm_iproc_i2c_dev *iproc_i2c,
+ u32 val)
+{
+ u8 err_status;
+ int ret = 0;
+
+ if (!iproc_i2c->en_s_pec)
+ return ret;
+
+ err_status = (u8)((val >> S_RX_PEC_ERR_SHIFT) & S_RX_PEC_ERR_MASK);
+ if (err_status == S_RX_PEC_ERR) {
+ dev_err(iproc_i2c->device, "Slave PEC error\n");
+ ret = -EBADMSG;
+ }
+
+ return ret;
+}
+
static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
u32 status)
{
@@ -347,6 +370,8 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
iproc_i2c_wr_reg(iproc_i2c, S_TX_OFFSET, value);
val = BIT(S_CMD_START_BUSY_SHIFT);
+ if (iproc_i2c->en_s_pec)
+ val |= BIT(S_CMD_PEC_SHIFT);
iproc_i2c_wr_reg(iproc_i2c, S_CMD_OFFSET, val);
/*
@@ -361,9 +386,19 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
value = (u8)((val >> S_RX_DATA_SHIFT) & S_RX_DATA_MASK);
i2c_slave_event(iproc_i2c->slave,
I2C_SLAVE_WRITE_RECEIVED, &value);
- if (rx_status == I2C_SLAVE_RX_END)
- i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_STOP, &value);
+ if (rx_status == I2C_SLAVE_RX_END) {
+ int ret;
+
+ ret = bcm_iproc_smbus_check_slave_pec(iproc_i2c,
+ val);
+ if (!ret)
+ i2c_slave_event(iproc_i2c->slave,
+ I2C_SLAVE_STOP, &value);
+ else
+ i2c_slave_event(iproc_i2c->slave,
+ I2C_SLAVE_PEC_ERR,
+ &value);
+ }
}
} else if (status & BIT(IS_S_TX_UNDERRUN_SHIFT)) {
/* Master read other than start */
@@ -372,6 +407,8 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
iproc_i2c_wr_reg(iproc_i2c, S_TX_OFFSET, value);
val = BIT(S_CMD_START_BUSY_SHIFT);
+ if (iproc_i2c->en_s_pec)
+ val |= BIT(S_CMD_PEC_SHIFT);
iproc_i2c_wr_reg(iproc_i2c, S_CMD_OFFSET, val);
}
@@ -1065,6 +1102,11 @@ static int bcm_iproc_i2c_reg_slave(struct i2c_client
*slave)
if (slave->flags & I2C_CLIENT_TEN)
return -EAFNOSUPPORT;
+ /* Enable partial slave HW PEC support if requested by the client */
+ iproc_i2c->en_s_pec = !!(slave->flags & I2C_CLIENT_PEC);
+ if (iproc_i2c->en_s_pec)
+ dev_info(iproc_i2c->device, "Enable PEC\n");
+
iproc_i2c->slave = slave;
bcm_iproc_i2c_slave_init(iproc_i2c, false);
return 0;
--
2.17.1
[PATCH V1 0/2] add PEC support on slave side
This patch set adds support for PEC on Slave side. Rayagonda Kokatanur (2): i2c: add PEC error event i2c: iproc: add slave pec support drivers/i2c/busses/i2c-bcm-iproc.c | 50 +++--- include/linux/i2c.h| 1 + 2 files changed, 47 insertions(+), 4 deletions(-) -- 2.17.1
Re: [PATCH v2 1/1] drivers: mtd: spi-nor: update read capabilities for w25q64 and s25fl064k
On Wed, Jul 1, 2020 at 12:21 PM wrote:
>
> On 5/29/20 10:16 AM, Rayagonda Kokatanur wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> >
> > Both w25q64 and s25fl064k nor flash support QUAD and DUAL read
> > command, hence update the same in flash_info table.
> >
> > This is tested on Broadcom Stingray SoC (bcm958742t).
> >
> > Signed-off-by: Rayagonda Kokatanur
> > ---
> > Changes from v1:
> > -Address review comments from Vignesh Raghavendra
> > Update commit message with testing details.
> >
> > drivers/mtd/spi-nor/spansion.c | 3 ++-
> > drivers/mtd/spi-nor/winbond.c | 3 ++-
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> > index 6756202ace4b..c91bbb8d9cd6 100644
> > --- a/drivers/mtd/spi-nor/spansion.c
> > +++ b/drivers/mtd/spi-nor/spansion.c
> > @@ -52,7 +52,8 @@ static const struct flash_info spansion_parts[] = {
> > SECT_4K | SPI_NOR_DUAL_READ |
> > SPI_NOR_QUAD_READ) },
> > { "s25fl016k", INFO(0xef4015, 0, 64 * 1024, 32,
> > SECT_4K | SPI_NOR_DUAL_READ |
> > SPI_NOR_QUAD_READ) },
> > - { "s25fl064k", INFO(0xef4017, 0, 64 * 1024, 128, SECT_4K) },
> > + { "s25fl064k", INFO(0xef4017, 0, 64 * 1024, 128,
> > +SECT_4K | SPI_NOR_DUAL_READ |
> > SPI_NOR_QUAD_READ) },
> > { "s25fl116k", INFO(0x014015, 0, 64 * 1024, 32,
> > SECT_4K | SPI_NOR_DUAL_READ |
> > SPI_NOR_QUAD_READ) },
> > { "s25fl132k", INFO(0x014016, 0, 64 * 1024, 64, SECT_4K) },
> > diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> > index 17deabad57e1..2028cab3eff9 100644
> > --- a/drivers/mtd/spi-nor/winbond.c
> > +++ b/drivers/mtd/spi-nor/winbond.c
> > @@ -39,7 +39,8 @@ static const struct flash_info winbond_parts[] = {
> > SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ
> > |
> > SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
> > { "w25x64", INFO(0xef3017, 0, 64 * 1024, 128, SECT_4K) },
> > - { "w25q64", INFO(0xef4017, 0, 64 * 1024, 128, SECT_4K) },
> > + { "w25q64", INFO(0xef4017, 0, 64 * 1024, 128,
> > +SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>
> I checked the winbond website and from all the w25q64 flashes, W25Q64JV-IQ/JQ
> and W25Q64FV share the 0xef4017 flash ID. Both support 0x3b and 0x6b commands,
> which is fine.
>
> I see that s25fl064k and w25q64 share the same flash ID. The search alg will
> return the first hit, so s25fl064k even for the winbond parts. What is the
> difference between s25fl064k and W25Q64JVQ/W25Q64FV?
I think both are from different vendor.
Please refer link for more info -
https://lore.kernel.org/patchwork/patch/628090/
Best regards,
Rayagonda
>
> Cheers,
> ta
[PATCH v2 1/1] drivers: mtd: spi-nor: update read capabilities for w25q64 and s25fl064k
Both w25q64 and s25fl064k nor flash support QUAD and DUAL read
command, hence update the same in flash_info table.
This is tested on Broadcom Stingray SoC (bcm958742t).
Signed-off-by: Rayagonda Kokatanur
---
Changes from v1:
-Address review comments from Vignesh Raghavendra
Update commit message with testing details.
drivers/mtd/spi-nor/spansion.c | 3 ++-
drivers/mtd/spi-nor/winbond.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 6756202ace4b..c91bbb8d9cd6 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -52,7 +52,8 @@ static const struct flash_info spansion_parts[] = {
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "s25fl016k", INFO(0xef4015, 0, 64 * 1024, 32,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
- { "s25fl064k", INFO(0xef4017, 0, 64 * 1024, 128, SECT_4K) },
+ { "s25fl064k", INFO(0xef4017, 0, 64 * 1024, 128,
+SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "s25fl116k", INFO(0x014015, 0, 64 * 1024, 32,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "s25fl132k", INFO(0x014016, 0, 64 * 1024, 64, SECT_4K) },
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 17deabad57e1..2028cab3eff9 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -39,7 +39,8 @@ static const struct flash_info winbond_parts[] = {
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
{ "w25x64", INFO(0xef3017, 0, 64 * 1024, 128, SECT_4K) },
- { "w25q64", INFO(0xef4017, 0, 64 * 1024, 128, SECT_4K) },
+ { "w25q64", INFO(0xef4017, 0, 64 * 1024, 128,
+SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "w25q64dw", INFO(0xef6017, 0, 64 * 1024, 128,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
--
2.17.1
[PATCH v1 1/1] drivers: mtd: spi-nor: update read capabilities for w25q64 and s25fl064k
Both w25q64 and s25fl064k nor flash support QUAD and DUAL read
command, hence update the same in flash_info table.
Signed-off-by: Rayagonda Kokatanur
---
drivers/mtd/spi-nor/spansion.c | 3 ++-
drivers/mtd/spi-nor/winbond.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 6756202ace4b..c91bbb8d9cd6 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -52,7 +52,8 @@ static const struct flash_info spansion_parts[] = {
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "s25fl016k", INFO(0xef4015, 0, 64 * 1024, 32,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
- { "s25fl064k", INFO(0xef4017, 0, 64 * 1024, 128, SECT_4K) },
+ { "s25fl064k", INFO(0xef4017, 0, 64 * 1024, 128,
+SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "s25fl116k", INFO(0x014015, 0, 64 * 1024, 32,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "s25fl132k", INFO(0x014016, 0, 64 * 1024, 64, SECT_4K) },
diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 17deabad57e1..2028cab3eff9 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -39,7 +39,8 @@ static const struct flash_info winbond_parts[] = {
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
{ "w25x64", INFO(0xef3017, 0, 64 * 1024, 128, SECT_4K) },
- { "w25q64", INFO(0xef4017, 0, 64 * 1024, 128, SECT_4K) },
+ { "w25q64", INFO(0xef4017, 0, 64 * 1024, 128,
+SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "w25q64dw", INFO(0xef6017, 0, 64 * 1024, 128,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },
--
2.17.1
Re: [PATCH v1 1/1] drivers: phy: sr-usb: do not use internal fsm for USB2 phy init
Hi Vinod,
On Tue, May 19, 2020 at 12:22 PM Vinod Koul wrote:
>
> On 13-05-20, 23:09, Rayagonda Kokatanur wrote:
> > From: Bharat Gooty
> >
> > During different reboot cycles, USB PHY PLL may not always lock
> > during initialization and therefore can cause USB to be not usable.
>
> Ok
>
> > Hence do not use internal FSM programming sequence for the USB
> > PHY initialization.
>
> And what is the impact of not using FSM programming sequence? If not
> impact why was it added in the first place ?
We have two methods for PHY bring-up. One is the current method and
the other is the FSM programming sequence. As we have observed PHY pll
lock is not happening after long reboots, we need to use the other
method. Using current method we have tested for 500,000 reboot
iterations and always USB PHY pll lock has happened. Connected USB
devices are detected and enumerated.
Best regards,
Rayagonda
>
> > Fixes: 4dcddbb38b64 ("phy: sr-usb: Add Stingray USB PHY driver")
> > Signed-off-by: Bharat Gooty
> > Signed-off-by: Rayagonda Kokatanur
> > ---
> > drivers/phy/broadcom/phy-bcm-sr-usb.c | 55 +--
> > 1 file changed, 2 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/phy/broadcom/phy-bcm-sr-usb.c
> > b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> > index fe6c58910e4c..7c7862b4f41f 100644
> > --- a/drivers/phy/broadcom/phy-bcm-sr-usb.c
> > +++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> > @@ -16,8 +16,6 @@ enum bcm_usb_phy_version {
> > };
> >
> > enum bcm_usb_phy_reg {
> > - PLL_NDIV_FRAC,
> > - PLL_NDIV_INT,
> > PLL_CTRL,
> > PHY_CTRL,
> > PHY_PLL_CTRL,
> > @@ -31,18 +29,11 @@ static const u8 bcm_usb_combo_phy_ss[] = {
> > };
> >
> > static const u8 bcm_usb_combo_phy_hs[] = {
> > - [PLL_NDIV_FRAC] = 0x04,
> > - [PLL_NDIV_INT] = 0x08,
> > [PLL_CTRL] = 0x0c,
> > [PHY_CTRL] = 0x10,
> > };
> >
> > -#define HSPLL_NDIV_INT_VAL 0x13
> > -#define HSPLL_NDIV_FRAC_VAL 0x1005
> > -
> > static const u8 bcm_usb_hs_phy[] = {
> > - [PLL_NDIV_FRAC] = 0x0,
> > - [PLL_NDIV_INT] = 0x4,
> > [PLL_CTRL] = 0x8,
> > [PHY_CTRL] = 0xc,
> > };
> > @@ -52,7 +43,6 @@ enum pll_ctrl_bits {
> > SSPLL_SUSPEND_EN,
> > PLL_SEQ_START,
> > PLL_LOCK,
> > - PLL_PDIV,
> > };
> >
> > static const u8 u3pll_ctrl[] = {
> > @@ -66,29 +56,17 @@ static const u8 u3pll_ctrl[] = {
> > #define HSPLL_PDIV_VAL 0x1
> >
> > static const u8 u2pll_ctrl[] = {
> > - [PLL_PDIV] = 1,
> > [PLL_RESETB]= 5,
> > [PLL_LOCK] = 6,
> > };
> >
> > enum bcm_usb_phy_ctrl_bits {
> > CORERDY,
> > - AFE_LDO_PWRDWNB,
> > - AFE_PLL_PWRDWNB,
> > - AFE_BG_PWRDWNB,
> > - PHY_ISO,
> > PHY_RESETB,
> > PHY_PCTL,
> > };
> >
> > #define PHY_PCTL_MASK0x
> > -/*
> > - * 0x0806 of PCTL_VAL has below bits set
> > - * BIT-8 : refclk divider 1
> > - * BIT-3:2: device mode; mode is not effect
> > - * BIT-1: soft reset active low
> > - */
> > -#define HSPHY_PCTL_VAL 0x0806
> > #define SSPHY_PCTL_VAL 0x0006
> >
> > static const u8 u3phy_ctrl[] = {
> > @@ -98,10 +76,6 @@ static const u8 u3phy_ctrl[] = {
> >
> > static const u8 u2phy_ctrl[] = {
> > [CORERDY] = 0,
> > - [AFE_LDO_PWRDWNB] = 1,
> > - [AFE_PLL_PWRDWNB] = 2,
> > - [AFE_BG_PWRDWNB]= 3,
> > - [PHY_ISO] = 4,
> > [PHY_RESETB]= 5,
> > [PHY_PCTL] = 6,
> > };
> > @@ -186,38 +160,13 @@ static int bcm_usb_hs_phy_init(struct bcm_usb_phy_cfg
> > *phy_cfg)
> > int ret = 0;
> > void __iomem *regs = phy_cfg->regs;
> > const u8 *offset;
> > - u32 rd_data;
> >
> > offset = phy_cfg->offset;
> >
> > - writel(HSPLL_NDIV_INT_VAL, regs + offset[PLL_NDIV_INT]);
> > - writel(HSPLL_NDIV_FRAC_VAL, regs + offset[PLL_NDIV_FRAC]);
> > -
> > - rd_data = readl(regs + offset[PLL_CTRL]);
> > - rd_data &= ~(HSPLL_PDIV_MASK << u2pll_ctrl[PLL_PDIV]);
> > - rd_data |= (HSPLL_PDIV_VAL << u2pll_ctrl[PLL_PDIV]);
> > - writel(rd_data, regs + offset[PLL_CTRL]);
> > -
> > - /* Set Core Ready high */
> > - bcm_usb_reg32_setbits(regs
[PATCH v1 1/1] drivers: phy: sr-usb: do not use internal fsm for USB2 phy init
From: Bharat Gooty
During different reboot cycles, USB PHY PLL may not always lock
during initialization and therefore can cause USB to be not usable.
Hence do not use internal FSM programming sequence for the USB
PHY initialization.
Fixes: 4dcddbb38b64 ("phy: sr-usb: Add Stingray USB PHY driver")
Signed-off-by: Bharat Gooty
Signed-off-by: Rayagonda Kokatanur
---
drivers/phy/broadcom/phy-bcm-sr-usb.c | 55 +--
1 file changed, 2 insertions(+), 53 deletions(-)
diff --git a/drivers/phy/broadcom/phy-bcm-sr-usb.c
b/drivers/phy/broadcom/phy-bcm-sr-usb.c
index fe6c58910e4c..7c7862b4f41f 100644
--- a/drivers/phy/broadcom/phy-bcm-sr-usb.c
+++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
@@ -16,8 +16,6 @@ enum bcm_usb_phy_version {
};
enum bcm_usb_phy_reg {
- PLL_NDIV_FRAC,
- PLL_NDIV_INT,
PLL_CTRL,
PHY_CTRL,
PHY_PLL_CTRL,
@@ -31,18 +29,11 @@ static const u8 bcm_usb_combo_phy_ss[] = {
};
static const u8 bcm_usb_combo_phy_hs[] = {
- [PLL_NDIV_FRAC] = 0x04,
- [PLL_NDIV_INT] = 0x08,
[PLL_CTRL] = 0x0c,
[PHY_CTRL] = 0x10,
};
-#define HSPLL_NDIV_INT_VAL 0x13
-#define HSPLL_NDIV_FRAC_VAL0x1005
-
static const u8 bcm_usb_hs_phy[] = {
- [PLL_NDIV_FRAC] = 0x0,
- [PLL_NDIV_INT] = 0x4,
[PLL_CTRL] = 0x8,
[PHY_CTRL] = 0xc,
};
@@ -52,7 +43,6 @@ enum pll_ctrl_bits {
SSPLL_SUSPEND_EN,
PLL_SEQ_START,
PLL_LOCK,
- PLL_PDIV,
};
static const u8 u3pll_ctrl[] = {
@@ -66,29 +56,17 @@ static const u8 u3pll_ctrl[] = {
#define HSPLL_PDIV_VAL 0x1
static const u8 u2pll_ctrl[] = {
- [PLL_PDIV] = 1,
[PLL_RESETB]= 5,
[PLL_LOCK] = 6,
};
enum bcm_usb_phy_ctrl_bits {
CORERDY,
- AFE_LDO_PWRDWNB,
- AFE_PLL_PWRDWNB,
- AFE_BG_PWRDWNB,
- PHY_ISO,
PHY_RESETB,
PHY_PCTL,
};
#define PHY_PCTL_MASK 0x
-/*
- * 0x0806 of PCTL_VAL has below bits set
- * BIT-8 : refclk divider 1
- * BIT-3:2: device mode; mode is not effect
- * BIT-1: soft reset active low
- */
-#define HSPHY_PCTL_VAL 0x0806
#define SSPHY_PCTL_VAL 0x0006
static const u8 u3phy_ctrl[] = {
@@ -98,10 +76,6 @@ static const u8 u3phy_ctrl[] = {
static const u8 u2phy_ctrl[] = {
[CORERDY] = 0,
- [AFE_LDO_PWRDWNB] = 1,
- [AFE_PLL_PWRDWNB] = 2,
- [AFE_BG_PWRDWNB]= 3,
- [PHY_ISO] = 4,
[PHY_RESETB]= 5,
[PHY_PCTL] = 6,
};
@@ -186,38 +160,13 @@ static int bcm_usb_hs_phy_init(struct bcm_usb_phy_cfg
*phy_cfg)
int ret = 0;
void __iomem *regs = phy_cfg->regs;
const u8 *offset;
- u32 rd_data;
offset = phy_cfg->offset;
- writel(HSPLL_NDIV_INT_VAL, regs + offset[PLL_NDIV_INT]);
- writel(HSPLL_NDIV_FRAC_VAL, regs + offset[PLL_NDIV_FRAC]);
-
- rd_data = readl(regs + offset[PLL_CTRL]);
- rd_data &= ~(HSPLL_PDIV_MASK << u2pll_ctrl[PLL_PDIV]);
- rd_data |= (HSPLL_PDIV_VAL << u2pll_ctrl[PLL_PDIV]);
- writel(rd_data, regs + offset[PLL_CTRL]);
-
- /* Set Core Ready high */
- bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
- BIT(u2phy_ctrl[CORERDY]));
-
- /* Maximum timeout for Core Ready done */
- msleep(30);
-
+ bcm_usb_reg32_clrbits(regs + offset[PLL_CTRL],
+ BIT(u2pll_ctrl[PLL_RESETB]));
bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
BIT(u2pll_ctrl[PLL_RESETB]));
- bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
- BIT(u2phy_ctrl[PHY_RESETB]));
-
-
- rd_data = readl(regs + offset[PHY_CTRL]);
- rd_data &= ~(PHY_PCTL_MASK << u2phy_ctrl[PHY_PCTL]);
- rd_data |= (HSPHY_PCTL_VAL << u2phy_ctrl[PHY_PCTL]);
- writel(rd_data, regs + offset[PHY_CTRL]);
-
- /* Maximum timeout for PLL reset done */
- msleep(30);
ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
BIT(u2pll_ctrl[PLL_LOCK]));
--
2.17.1
Re: [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
Hi Wolfram,
Please review the patch and let me know if you still have any review comments.
Best regards,
Rayagonda
On Thu, Oct 10, 2019 at 3:02 PM Rayagonda Kokatanur
wrote:
>
> Hi Wolfram,
>
> Did you get a chance to review this patch.
>
> Best regards,
> Rayagonda
>
>
> On Mon, Sep 30, 2019 at 12:19 PM Rayagonda Kokatanur
> wrote:
> >
> > From: Lori Hikichi
> >
> > Enable handling of i2c repeated start. The current code
> > handles a multi msg i2c transfer as separate i2c bus
> > transactions. This change will now handle this case
> > using the i2c repeated start protocol. The number of msgs
> > in a transfer is limited to two, and must be a write
> > followed by a read.
> >
> > Signed-off-by: Lori Hikichi
> > Signed-off-by: Rayagonda Kokatanur
> > Signed-off-by: Icarus Chau
> > Signed-off-by: Ray Jui
> > Signed-off-by: Shivaraj Shetty
> > ---
> > changes from v1:
> > - Address following review comments from Wolfarm Sang,
> >Use i2c_8bit_addr_from_msg() api instead of decoding i2c_msg struct and
> >remove check against number of i2c message as it will be taken care
> >by core using quirks flags.
> >
> > drivers/i2c/busses/i2c-bcm-iproc.c | 63
> > ++
> > 1 file changed, 50 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
> > b/drivers/i2c/busses/i2c-bcm-iproc.c
> > index d7fd76b..e478db7 100644
> > --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> > +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> > @@ -81,6 +81,7 @@
> > #define M_CMD_PROTOCOL_MASK 0xf
> > #define M_CMD_PROTOCOL_BLK_WR0x7
> > #define M_CMD_PROTOCOL_BLK_RD0x8
> > +#define M_CMD_PROTOCOL_PROCESS 0xa
> > #define M_CMD_PEC_SHIFT 8
> > #define M_CMD_RD_CNT_SHIFT 0
> > #define M_CMD_RD_CNT_MASK0xff
> > @@ -675,13 +676,20 @@ static int bcm_iproc_i2c_xfer_wait(struct
> > bcm_iproc_i2c_dev *iproc_i2c,
> > return 0;
> > }
> >
> > -static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev
> > *iproc_i2c,
> > -struct i2c_msg *msg)
> > +/*
> > + * If 'process_call' is true, then this is a multi-msg transfer that
> > requires
> > + * a repeated start between the messages.
> > + * More specifically, it must be a write (reg) followed by a read (data).
> > + * The i2c quirks are set to enforce this rule.
> > + */
> > +static int bcm_iproc_i2c_xfer_internal(struct bcm_iproc_i2c_dev *iproc_i2c,
> > + struct i2c_msg *msgs, bool
> > process_call)
> > {
> > int i;
> > u8 addr;
> > u32 val, tmp, val_intr_en;
> > unsigned int tx_bytes;
> > + struct i2c_msg *msg = &msgs[0];
> >
> > /* check if bus is busy */
> > if (!!(iproc_i2c_rd_reg(iproc_i2c,
> > @@ -707,14 +715,29 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
> > bcm_iproc_i2c_dev *iproc_i2c,
> > val = msg->buf[i];
> >
> > /* mark the last byte */
> > - if (i == msg->len - 1)
> > - val |= BIT(M_TX_WR_STATUS_SHIFT);
> > + if (!process_call && (i == msg->len - 1))
> > + val |= 1 << M_TX_WR_STATUS_SHIFT;
> >
> > iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
> > }
> > iproc_i2c->tx_bytes = tx_bytes;
> > }
> >
> > + /* Process the read message if this is process call */
> > + if (process_call) {
> > + msg++;
> > + iproc_i2c->msg = msg; /* point to second msg */
> > +
> > + /*
> > +* The last byte to be sent out should be a slave
> > +* address with read operation
> > +*/
> > + addr = i2c_8bit_addr_from_msg(msg);
> > + /* mark it the last byte out */
> > + val = addr | (1 << M_TX_WR_STATUS_SHIFT);
> > + iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
> > + }
> > +
> > /* mark as incomplete before starting the transaction */
> > if (iproc_i2c->irq)
> > reinit_completion(&iproc_i2c->done);
> > @@ -733,7 +756,7
Re: [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
Hi Wolfram,
Did you get a chance to review this patch.
Best regards,
Rayagonda
On Mon, Sep 30, 2019 at 12:19 PM Rayagonda Kokatanur
wrote:
>
> From: Lori Hikichi
>
> Enable handling of i2c repeated start. The current code
> handles a multi msg i2c transfer as separate i2c bus
> transactions. This change will now handle this case
> using the i2c repeated start protocol. The number of msgs
> in a transfer is limited to two, and must be a write
> followed by a read.
>
> Signed-off-by: Lori Hikichi
> Signed-off-by: Rayagonda Kokatanur
> Signed-off-by: Icarus Chau
> Signed-off-by: Ray Jui
> Signed-off-by: Shivaraj Shetty
> ---
> changes from v1:
> - Address following review comments from Wolfarm Sang,
>Use i2c_8bit_addr_from_msg() api instead of decoding i2c_msg struct and
>remove check against number of i2c message as it will be taken care
>by core using quirks flags.
>
> drivers/i2c/busses/i2c-bcm-iproc.c | 63
> ++
> 1 file changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
> b/drivers/i2c/busses/i2c-bcm-iproc.c
> index d7fd76b..e478db7 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -81,6 +81,7 @@
> #define M_CMD_PROTOCOL_MASK 0xf
> #define M_CMD_PROTOCOL_BLK_WR0x7
> #define M_CMD_PROTOCOL_BLK_RD0x8
> +#define M_CMD_PROTOCOL_PROCESS 0xa
> #define M_CMD_PEC_SHIFT 8
> #define M_CMD_RD_CNT_SHIFT 0
> #define M_CMD_RD_CNT_MASK0xff
> @@ -675,13 +676,20 @@ static int bcm_iproc_i2c_xfer_wait(struct
> bcm_iproc_i2c_dev *iproc_i2c,
> return 0;
> }
>
> -static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
> -struct i2c_msg *msg)
> +/*
> + * If 'process_call' is true, then this is a multi-msg transfer that requires
> + * a repeated start between the messages.
> + * More specifically, it must be a write (reg) followed by a read (data).
> + * The i2c quirks are set to enforce this rule.
> + */
> +static int bcm_iproc_i2c_xfer_internal(struct bcm_iproc_i2c_dev *iproc_i2c,
> + struct i2c_msg *msgs, bool
> process_call)
> {
> int i;
> u8 addr;
> u32 val, tmp, val_intr_en;
> unsigned int tx_bytes;
> + struct i2c_msg *msg = &msgs[0];
>
> /* check if bus is busy */
> if (!!(iproc_i2c_rd_reg(iproc_i2c,
> @@ -707,14 +715,29 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
> bcm_iproc_i2c_dev *iproc_i2c,
> val = msg->buf[i];
>
> /* mark the last byte */
> - if (i == msg->len - 1)
> - val |= BIT(M_TX_WR_STATUS_SHIFT);
> + if (!process_call && (i == msg->len - 1))
> + val |= 1 << M_TX_WR_STATUS_SHIFT;
>
> iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
> }
> iproc_i2c->tx_bytes = tx_bytes;
> }
>
> + /* Process the read message if this is process call */
> + if (process_call) {
> + msg++;
> + iproc_i2c->msg = msg; /* point to second msg */
> +
> + /*
> +* The last byte to be sent out should be a slave
> +* address with read operation
> +*/
> + addr = i2c_8bit_addr_from_msg(msg);
> + /* mark it the last byte out */
> + val = addr | (1 << M_TX_WR_STATUS_SHIFT);
> + iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
> + }
> +
> /* mark as incomplete before starting the transaction */
> if (iproc_i2c->irq)
> reinit_completion(&iproc_i2c->done);
> @@ -733,7 +756,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
> bcm_iproc_i2c_dev *iproc_i2c,
> * underrun interrupt, which will be triggerred when the TX FIFO is
> * empty. When that happens we can then pump more data into the FIFO
> */
> - if (!(msg->flags & I2C_M_RD) &&
> + if (!process_call && !(msg->flags & I2C_M_RD) &&
> msg->len > iproc_i2c->tx_bytes)
> val_intr_en |= BIT(IE_M_TX_UNDERRUN_SHIFT);
>
> @@ -743,6 +766,8 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
> bcm_iproc_i2c_dev *iproc_i2c,
> */
> val = BIT(M_CMD_START_BUSY_SHIFT);
> if (msg->fl
Re: [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
On Sat, Sep 28, 2019 at 11:53 PM Wolfram Sang wrote: > > On Thu, Sep 26, 2019 at 10:10:08AM +0530, Rayagonda Kokatanur wrote: > > From: Lori Hikichi > > > > Enable handling of i2c repeated start. The current code > > handles a multi msg i2c transfer as separate i2c bus > > transactions. This change will now handle this case > > using the i2c repeated start protocol. The number of msgs > > in a transfer is limited to two, and must be a write > > followed by a read. > > > > Signed-off-by: Lori Hikichi > > Signed-off-by: Rayagonda Kokatanur > > Signed-off-by: Icarus Chau > > Signed-off-by: Ray Jui > > Signed-off-by: Shivaraj Shetty > > --- > > changes from v1: > > - Address code review comment from Wolfram Sang > > No, sorry, this is not a proper changelog. I review so many patches, I > can't recall what I suggested to do for every patch. Please describe > what changes you actually made. It is also better when digging through > mail archives. > Sorry for inconvenience, I updated the changelog and resent the patch. I have kept the patch version as v2 only. Hope that is fine. Best regards, Rayagonda
[PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
From: Lori Hikichi
Enable handling of i2c repeated start. The current code
handles a multi msg i2c transfer as separate i2c bus
transactions. This change will now handle this case
using the i2c repeated start protocol. The number of msgs
in a transfer is limited to two, and must be a write
followed by a read.
Signed-off-by: Lori Hikichi
Signed-off-by: Rayagonda Kokatanur
Signed-off-by: Icarus Chau
Signed-off-by: Ray Jui
Signed-off-by: Shivaraj Shetty
---
changes from v1:
- Address following review comments from Wolfarm Sang,
Use i2c_8bit_addr_from_msg() api instead of decoding i2c_msg struct and
remove check against number of i2c message as it will be taken care
by core using quirks flags.
drivers/i2c/busses/i2c-bcm-iproc.c | 63 ++
1 file changed, 50 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index d7fd76b..e478db7 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -81,6 +81,7 @@
#define M_CMD_PROTOCOL_MASK 0xf
#define M_CMD_PROTOCOL_BLK_WR0x7
#define M_CMD_PROTOCOL_BLK_RD0x8
+#define M_CMD_PROTOCOL_PROCESS 0xa
#define M_CMD_PEC_SHIFT 8
#define M_CMD_RD_CNT_SHIFT 0
#define M_CMD_RD_CNT_MASK0xff
@@ -675,13 +676,20 @@ static int bcm_iproc_i2c_xfer_wait(struct
bcm_iproc_i2c_dev *iproc_i2c,
return 0;
}
-static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
-struct i2c_msg *msg)
+/*
+ * If 'process_call' is true, then this is a multi-msg transfer that requires
+ * a repeated start between the messages.
+ * More specifically, it must be a write (reg) followed by a read (data).
+ * The i2c quirks are set to enforce this rule.
+ */
+static int bcm_iproc_i2c_xfer_internal(struct bcm_iproc_i2c_dev *iproc_i2c,
+ struct i2c_msg *msgs, bool process_call)
{
int i;
u8 addr;
u32 val, tmp, val_intr_en;
unsigned int tx_bytes;
+ struct i2c_msg *msg = &msgs[0];
/* check if bus is busy */
if (!!(iproc_i2c_rd_reg(iproc_i2c,
@@ -707,14 +715,29 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
val = msg->buf[i];
/* mark the last byte */
- if (i == msg->len - 1)
- val |= BIT(M_TX_WR_STATUS_SHIFT);
+ if (!process_call && (i == msg->len - 1))
+ val |= 1 << M_TX_WR_STATUS_SHIFT;
iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
}
iproc_i2c->tx_bytes = tx_bytes;
}
+ /* Process the read message if this is process call */
+ if (process_call) {
+ msg++;
+ iproc_i2c->msg = msg; /* point to second msg */
+
+ /*
+* The last byte to be sent out should be a slave
+* address with read operation
+*/
+ addr = i2c_8bit_addr_from_msg(msg);
+ /* mark it the last byte out */
+ val = addr | (1 << M_TX_WR_STATUS_SHIFT);
+ iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
+ }
+
/* mark as incomplete before starting the transaction */
if (iproc_i2c->irq)
reinit_completion(&iproc_i2c->done);
@@ -733,7 +756,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
* underrun interrupt, which will be triggerred when the TX FIFO is
* empty. When that happens we can then pump more data into the FIFO
*/
- if (!(msg->flags & I2C_M_RD) &&
+ if (!process_call && !(msg->flags & I2C_M_RD) &&
msg->len > iproc_i2c->tx_bytes)
val_intr_en |= BIT(IE_M_TX_UNDERRUN_SHIFT);
@@ -743,6 +766,8 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
*/
val = BIT(M_CMD_START_BUSY_SHIFT);
if (msg->flags & I2C_M_RD) {
+ u32 protocol;
+
iproc_i2c->rx_bytes = 0;
if (msg->len > M_RX_FIFO_MAX_THLD_VALUE)
iproc_i2c->thld_bytes = M_RX_FIFO_THLD_VALUE;
@@ -758,7 +783,10 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
/* enable the RX threshold interrupt */
val_intr_en |= BIT(IE_M_RX_THLD_SHIFT);
- val |= (M_CMD_PROTOCOL_BLK_RD << M_CMD_PROTOCOL_SHIFT) |
+ protocol = process_call ?
+ M_CMD_PROTOCOL_PROCESS : M_CMD_PROTOCOL_BLK_RD;
+
+ val |=
Re: [PATCH] dmaengine: bcm-sba-raid: Handle mbox_request_channel failure
Hi Vinod,
Did you get chance to review this fix?
Best regards,
Rayagonda
On Thu, Jan 10, 2019 at 11:06 PM Ray Jui wrote:
>
>
>
> On 1/9/2019 10:07 PM, Rayagonda Kokatanur wrote:
> > Fix kernel NULL pointer dereference error when mbox_request_channel()
> > fails to allocate channel.
> >
> > Fixes: 4e9f8187aecb ("dmaengine: bcm-sba-raid: Use only single mailbox
> > channel")
> > Signed-off-by: Rayagonda Kokatanur
> > ---
> > drivers/dma/bcm-sba-raid.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
> > index 72878ac5c78d..77ae74663a45 100644
> > --- a/drivers/dma/bcm-sba-raid.c
> > +++ b/drivers/dma/bcm-sba-raid.c
> > @@ -1690,7 +1690,7 @@ static int sba_probe(struct platform_device *pdev)
> > sba->mchan = mbox_request_channel(&sba->client, 0);
> > if (IS_ERR(sba->mchan)) {
> > ret = PTR_ERR(sba->mchan);
> > - goto fail_free_mchan;
> > + goto fail_exit;
> > }
> >
> > /* Find-out underlying mailbox device */
> > @@ -1747,6 +1747,7 @@ static int sba_probe(struct platform_device *pdev)
> > sba_freeup_channel_resources(sba);
> > fail_free_mchan:
> > mbox_free_channel(sba->mchan);
> > +fail_exit:
> > return ret;
> > }
> >
> >
>
> Looks good to me.
>
> Reviewed-by: Ray Jui
[PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
From: Lori Hikichi
Enable handling of i2c repeated start. The current code
handles a multi msg i2c transfer as separate i2c bus
transactions. This change will now handle this case
using the i2c repeated start protocol. The number of msgs
in a transfer is limited to two, and must be a write
followed by a read.
Signed-off-by: Lori Hikichi
Signed-off-by: Rayagonda Kokatanur
Signed-off-by: Icarus Chau
Signed-off-by: Ray Jui
Signed-off-by: Shivaraj Shetty
---
changes from v1:
- Address code review comment from Wolfram Sang
drivers/i2c/busses/i2c-bcm-iproc.c | 63 ++
1 file changed, 50 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index d7fd76b..e478db7 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -81,6 +81,7 @@
#define M_CMD_PROTOCOL_MASK 0xf
#define M_CMD_PROTOCOL_BLK_WR0x7
#define M_CMD_PROTOCOL_BLK_RD0x8
+#define M_CMD_PROTOCOL_PROCESS 0xa
#define M_CMD_PEC_SHIFT 8
#define M_CMD_RD_CNT_SHIFT 0
#define M_CMD_RD_CNT_MASK0xff
@@ -675,13 +676,20 @@ static int bcm_iproc_i2c_xfer_wait(struct
bcm_iproc_i2c_dev *iproc_i2c,
return 0;
}
-static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
-struct i2c_msg *msg)
+/*
+ * If 'process_call' is true, then this is a multi-msg transfer that requires
+ * a repeated start between the messages.
+ * More specifically, it must be a write (reg) followed by a read (data).
+ * The i2c quirks are set to enforce this rule.
+ */
+static int bcm_iproc_i2c_xfer_internal(struct bcm_iproc_i2c_dev *iproc_i2c,
+ struct i2c_msg *msgs, bool process_call)
{
int i;
u8 addr;
u32 val, tmp, val_intr_en;
unsigned int tx_bytes;
+ struct i2c_msg *msg = &msgs[0];
/* check if bus is busy */
if (!!(iproc_i2c_rd_reg(iproc_i2c,
@@ -707,14 +715,29 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
val = msg->buf[i];
/* mark the last byte */
- if (i == msg->len - 1)
- val |= BIT(M_TX_WR_STATUS_SHIFT);
+ if (!process_call && (i == msg->len - 1))
+ val |= 1 << M_TX_WR_STATUS_SHIFT;
iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
}
iproc_i2c->tx_bytes = tx_bytes;
}
+ /* Process the read message if this is process call */
+ if (process_call) {
+ msg++;
+ iproc_i2c->msg = msg; /* point to second msg */
+
+ /*
+* The last byte to be sent out should be a slave
+* address with read operation
+*/
+ addr = i2c_8bit_addr_from_msg(msg);
+ /* mark it the last byte out */
+ val = addr | (1 << M_TX_WR_STATUS_SHIFT);
+ iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
+ }
+
/* mark as incomplete before starting the transaction */
if (iproc_i2c->irq)
reinit_completion(&iproc_i2c->done);
@@ -733,7 +756,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
* underrun interrupt, which will be triggerred when the TX FIFO is
* empty. When that happens we can then pump more data into the FIFO
*/
- if (!(msg->flags & I2C_M_RD) &&
+ if (!process_call && !(msg->flags & I2C_M_RD) &&
msg->len > iproc_i2c->tx_bytes)
val_intr_en |= BIT(IE_M_TX_UNDERRUN_SHIFT);
@@ -743,6 +766,8 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
*/
val = BIT(M_CMD_START_BUSY_SHIFT);
if (msg->flags & I2C_M_RD) {
+ u32 protocol;
+
iproc_i2c->rx_bytes = 0;
if (msg->len > M_RX_FIFO_MAX_THLD_VALUE)
iproc_i2c->thld_bytes = M_RX_FIFO_THLD_VALUE;
@@ -758,7 +783,10 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
/* enable the RX threshold interrupt */
val_intr_en |= BIT(IE_M_RX_THLD_SHIFT);
- val |= (M_CMD_PROTOCOL_BLK_RD << M_CMD_PROTOCOL_SHIFT) |
+ protocol = process_call ?
+ M_CMD_PROTOCOL_PROCESS : M_CMD_PROTOCOL_BLK_RD;
+
+ val |= (protocol << M_CMD_PROTOCOL_SHIFT) |
(msg->len << M_CMD_RD_CNT_SHIFT);
} else {
val |= (M_CMD_PROTOCOL_BLK_WR << M_CMD_PROTOC
[PATCH v1 1/1] pinctrl: iproc: Add 'get_direction' support
Add 'get_direction' support to the iProc GPIO driver.
Signed-off-by: Rayagonda Kokatanur
---
drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
index b70058c..d782d70 100644
--- a/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
+++ b/drivers/pinctrl/bcm/pinctrl-iproc-gpio.c
@@ -355,6 +355,15 @@ static int iproc_gpio_direction_output(struct gpio_chip
*gc, unsigned gpio,
return 0;
}
+static int iproc_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct iproc_gpio *chip = gpiochip_get_data(gc);
+ unsigned int offset = IPROC_GPIO_REG(gpio, IPROC_GPIO_OUT_EN_OFFSET);
+ unsigned int shift = IPROC_GPIO_SHIFT(gpio);
+
+ return !(readl(chip->base + offset) & BIT(shift));
+}
+
static void iproc_gpio_set(struct gpio_chip *gc, unsigned gpio, int val)
{
struct iproc_gpio *chip = gpiochip_get_data(gc);
@@ -784,6 +793,7 @@ static int iproc_gpio_probe(struct platform_device *pdev)
gc->free = iproc_gpio_free;
gc->direction_input = iproc_gpio_direction_input;
gc->direction_output = iproc_gpio_direction_output;
+ gc->get_direction = iproc_gpio_get_direction;
gc->set = iproc_gpio_set;
gc->get = iproc_gpio_get;
--
1.9.1
[PATCH v1 1/1] arm64: dts: Fix gpio to pinmux mapping
There are total of 151 non-secure gpio (0-150) and four
pins of pinmux (91, 92, 93 and 94) are not mapped to any
gpio pin, hence update same in DT.
Fixes: 8aa428cc1e2e ("arm64: dts: Add pinctrl DT nodes for Stingray SOC")
Signed-off-by: Rayagonda Kokatanur
---
arch/arm64/boot/dts/broadcom/stingray/stingray-pinctrl.dtsi | 5 +++--
arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi | 3 +--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray-pinctrl.dtsi
b/arch/arm64/boot/dts/broadcom/stingray/stingray-pinctrl.dtsi
index 8a3a770..56789cc 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray-pinctrl.dtsi
@@ -42,13 +42,14 @@
pinmux: pinmux@14029c {
compatible = "pinctrl-single";
- reg = <0x0014029c 0x250>;
+ reg = <0x0014029c 0x26c>;
#address-cells = <1>;
#size-cells = <1>;
pinctrl-single,register-width = <32>;
pinctrl-single,function-mask = <0xf>;
pinctrl-single,gpio-range = <
- &range 0 154 MODE_GPIO
+ &range 0 91 MODE_GPIO
+ &range 95 60 MODE_GPIO
>;
range: gpio-range {
#pinctrl-single,gpio-range-cells = <3>;
diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
index 71e2e34..0098dfd 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
@@ -464,8 +464,7 @@
<&pinmux 108 16 27>,
<&pinmux 135 77 6>,
<&pinmux 141 67 4>,
- <&pinmux 145 149 6>,
- <&pinmux 151 91 4>;
+ <&pinmux 145 149 6>;
};
i2c1: i2c@e {
--
1.9.1
Re: [PATCH V2 1/1] spi: bcm-qspi: Make BSPI default mode
On Thu, Aug 29, 2019 at 3:29 PM Mark Brown wrote: > > On Thu, Aug 29, 2019 at 09:46:13AM +0530, Rayagonda Kokatanur wrote: > > The spi-nor controller defaults to BSPI mode, hence switch back > > to its default mode after MSPI operations (write or erase) > > are completed. > > > > Changes from V1: > > - Address code review comment from Mark Brown. > > As covered in submitting-patches.rst inter-version changelogs should go > after the --- so they don't end up in git. Thank you, I fixed it and sent patch v3 for review.
[PATCH V3 1/1] spi: bcm-qspi: Make BSPI default mode
The spi-nor controller defaults to BSPI mode, hence switch back to its default mode after MSPI operations (write or erase) are completed. Signed-off-by: Rayagonda Kokatanur Reviewed-by: Mark Brown Reviewed-by: Kamal Dasu --- changes from V2: - Address code review comment from Mark Brown about changelog. Changes from V1: - Address code review comment from Mark Brown. drivers/spi/spi-bcm-qspi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c index 902bdbf..46a811a 100644 --- a/drivers/spi/spi-bcm-qspi.c +++ b/drivers/spi/spi-bcm-qspi.c @@ -897,6 +897,7 @@ static int bcm_qspi_transfer_one(struct spi_master *master, read_from_hw(qspi, slots); } + bcm_qspi_enable_bspi(qspi); return 0; } -- 1.9.1
[PATCH V2 1/1] spi: bcm-qspi: Make BSPI default mode
The spi-nor controller defaults to BSPI mode, hence switch back to its default mode after MSPI operations (write or erase) are completed. Changes from V1: - Address code review comment from Mark Brown. Signed-off-by: Rayagonda Kokatanur Reviewed-by: Mark Brown Reviewed-by: Kamal Dasu --- drivers/spi/spi-bcm-qspi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c index 902bdbf..46a811a 100644 --- a/drivers/spi/spi-bcm-qspi.c +++ b/drivers/spi/spi-bcm-qspi.c @@ -897,6 +897,7 @@ static int bcm_qspi_transfer_one(struct spi_master *master, read_from_hw(qspi, slots); } + bcm_qspi_enable_bspi(qspi); return 0; } -- 1.9.1
[PATCH v1 2/2] i2c: iproc: Add full name of devicetree node to adapter name
From: Lori Hikichi Add the full name of the devicetree node to the adapter name. Without this change, all adapters have the same name making it difficult to distinguish between multiple instances. The most obvious way to see this is to use the utility i2c_detect. e.g. "i2c-detect -l" Before i2c-1 i2c Broadcom iProc I2C adapter I2C adapter i2c-0 i2c Broadcom iProc I2C adapter I2C adapter After i2c-1 i2c Broadcom iProc (i2c@e) I2C adapter i2c-0 i2c Broadcom iProc (i2c@b) I2C adapter Now it is easy to figure out which adapter maps to a which DT node. Signed-off-by: Lori Hikichi Signed-off-by: Rayagonda Kokatanur --- drivers/i2c/busses/i2c-bcm-iproc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c index 19ef2b0..183b220 100644 --- a/drivers/i2c/busses/i2c-bcm-iproc.c +++ b/drivers/i2c/busses/i2c-bcm-iproc.c @@ -922,7 +922,9 @@ static int bcm_iproc_i2c_probe(struct platform_device *pdev) adap = &iproc_i2c->adapter; i2c_set_adapdata(adap, iproc_i2c); - strlcpy(adap->name, "Broadcom iProc I2C adapter", sizeof(adap->name)); + snprintf(adap->name, sizeof(adap->name), + "Broadcom iProc (%s)", + of_node_full_name(iproc_i2c->device->of_node)); adap->algo = &bcm_iproc_algo; adap->quirks = &bcm_iproc_i2c_quirks; adap->dev.parent = &pdev->dev; -- 1.9.1
[PATCH v1 1/2] i2c: iproc: Stop advertising support of SMBUS quick cmd
From: Lori Hikichi
The driver does not support the SMBUS Quick command so remove the
flag that indicates that level of support.
By default the i2c_detect tool uses the quick command to try and
detect devices at some bus addresses. If the quick command is used
then we will not detect the device, even though it is present.
Fixes: e6e5dd3566e0 (i2c: iproc: Add Broadcom iProc I2C Driver)
Signed-off-by: Lori Hikichi
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index d7fd76b..19ef2b0 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -790,7 +790,10 @@ static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
static uint32_t bcm_iproc_i2c_functionality(struct i2c_adapter *adap)
{
- u32 val = I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+ u32 val;
+
+ /* We do not support the SMBUS Quick command */
+ val = I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK);
if (adap->algo->reg_slave)
val |= I2C_FUNC_SLAVE;
--
1.9.1
[PATCH v1 0/2] Remove smbus quick cmd and update adapter name
Hi, This patchset contains following changes: - Remove SMBUS quick command support - Update full name of dt node to adapter name Lori Hikichi (2): i2c: iproc: Stop advertising support of SMBUS quick cmd i2c: iproc: Add full name of devicetree node to adapter name drivers/i2c/busses/i2c-bcm-iproc.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- 1.9.1
[PATCH v1 1/1] i2c: iproc: Add i2c repeated start capability
From: Lori Hikichi
Enable handling of i2c repeated start. The current code
handles a multi msg i2c transfer as separate i2c bus
transactions. This change will now handle this case
using the i2c repeated start protocol. The number of msgs
in a transfer is limited to two, and must be a write
followed by a read.
Signed-off-by: Lori Hikichi
Signed-off-by: Rayagonda Kokatanur
Signed-off-by: Icarus Chau
Signed-off-by: Ray Jui
Signed-off-by: Shivaraj Shetty
---
drivers/i2c/busses/i2c-bcm-iproc.c | 70 +++---
1 file changed, 57 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index d7fd76b..15fedcf 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -81,6 +81,7 @@
#define M_CMD_PROTOCOL_MASK 0xf
#define M_CMD_PROTOCOL_BLK_WR0x7
#define M_CMD_PROTOCOL_BLK_RD0x8
+#define M_CMD_PROTOCOL_PROCESS 0xa
#define M_CMD_PEC_SHIFT 8
#define M_CMD_RD_CNT_SHIFT 0
#define M_CMD_RD_CNT_MASK0xff
@@ -675,13 +676,20 @@ static int bcm_iproc_i2c_xfer_wait(struct
bcm_iproc_i2c_dev *iproc_i2c,
return 0;
}
-static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
-struct i2c_msg *msg)
+/*
+ * If 'process_call' is true, then this is a multi-msg transfer that requires
+ * a repeated start between the messages.
+ * More specifically, it must be a write (reg) followed by a read (data).
+ * The i2c quirks are set to enforce this rule.
+ */
+static int bcm_iproc_i2c_xfer_internal(struct bcm_iproc_i2c_dev *iproc_i2c,
+ struct i2c_msg *msgs, bool process_call)
{
int i;
u8 addr;
u32 val, tmp, val_intr_en;
unsigned int tx_bytes;
+ struct i2c_msg *msg = &msgs[0];
/* check if bus is busy */
if (!!(iproc_i2c_rd_reg(iproc_i2c,
@@ -707,14 +715,29 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
val = msg->buf[i];
/* mark the last byte */
- if (i == msg->len - 1)
- val |= BIT(M_TX_WR_STATUS_SHIFT);
+ if (!process_call && (i == msg->len - 1))
+ val |= 1 << M_TX_WR_STATUS_SHIFT;
iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
}
iproc_i2c->tx_bytes = tx_bytes;
}
+ /* Process the read message if this is process call */
+ if (process_call) {
+ msg++;
+ iproc_i2c->msg = msg; /* point to second msg */
+
+ /*
+* The last byte to be sent out should be a slave
+* address with read operation
+*/
+ addr = msg->addr << 1 | 1;
+ /* mark it the last byte out */
+ val = addr | (1 << M_TX_WR_STATUS_SHIFT);
+ iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
+ }
+
/* mark as incomplete before starting the transaction */
if (iproc_i2c->irq)
reinit_completion(&iproc_i2c->done);
@@ -733,7 +756,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
* underrun interrupt, which will be triggerred when the TX FIFO is
* empty. When that happens we can then pump more data into the FIFO
*/
- if (!(msg->flags & I2C_M_RD) &&
+ if (!process_call && !(msg->flags & I2C_M_RD) &&
msg->len > iproc_i2c->tx_bytes)
val_intr_en |= BIT(IE_M_TX_UNDERRUN_SHIFT);
@@ -743,6 +766,8 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
*/
val = BIT(M_CMD_START_BUSY_SHIFT);
if (msg->flags & I2C_M_RD) {
+ u32 protocol;
+
iproc_i2c->rx_bytes = 0;
if (msg->len > M_RX_FIFO_MAX_THLD_VALUE)
iproc_i2c->thld_bytes = M_RX_FIFO_THLD_VALUE;
@@ -758,7 +783,10 @@ static int bcm_iproc_i2c_xfer_single_msg(struct
bcm_iproc_i2c_dev *iproc_i2c,
/* enable the RX threshold interrupt */
val_intr_en |= BIT(IE_M_RX_THLD_SHIFT);
- val |= (M_CMD_PROTOCOL_BLK_RD << M_CMD_PROTOCOL_SHIFT) |
+ protocol = process_call ?
+ M_CMD_PROTOCOL_PROCESS : M_CMD_PROTOCOL_BLK_RD;
+
+ val |= (protocol << M_CMD_PROTOCOL_SHIFT) |
(msg->len << M_CMD_RD_CNT_SHIFT);
} else {
val |= (M_CMD_PROTOCOL_BLK_WR << M_CMD_PROTOCOL_SHIFT);
@@ -774,17 +802,31 @@ static int bcm_iproc_i2c_xfer(struct i2c_adapte
[PATCH v1 1/1] spi: bcm-qspi: Make BSPI default mode
Switch back to BSPI mode after MSPI operations (write and erase) are completed. This change will keep qpsi in BSPI mode by default. Signed-off-by: Rayagonda Kokatanur --- drivers/spi/spi-bcm-qspi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c index 902bdbf..46a811a 100644 --- a/drivers/spi/spi-bcm-qspi.c +++ b/drivers/spi/spi-bcm-qspi.c @@ -897,6 +897,7 @@ static int bcm_qspi_transfer_one(struct spi_master *master, read_from_hw(qspi, slots); } + bcm_qspi_enable_bspi(qspi); return 0; } -- 1.9.1
[PATCH v1 1/1] spi: bcm-qspi: Fix BSPI QUAD and DUAL mode support when using flex mode
Fix data transfer width settings based on DT field 'spi-rx-bus-width'
to configure BSPI in single, dual or quad mode by using data width
and not the command width.
Fixes: 5f195ee7d830c ("spi: bcm-qspi: Implement the spi_mem interface")
Signed-off-by: Rayagonda Kokatanur
---
drivers/spi/spi-bcm-qspi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c
index 902bdbf..0dbfd24 100644
--- a/drivers/spi/spi-bcm-qspi.c
+++ b/drivers/spi/spi-bcm-qspi.c
@@ -343,7 +343,7 @@ static int bcm_qspi_bspi_set_flex_mode(struct bcm_qspi
*qspi,
{
int bpc = 0, bpp = 0;
u8 command = op->cmd.opcode;
- int width = op->cmd.buswidth ? op->cmd.buswidth : SPI_NBITS_SINGLE;
+ int width = op->data.buswidth ? op->data.buswidth : SPI_NBITS_SINGLE;
int addrlen = op->addr.nbytes;
int flex_mode = 1;
@@ -981,7 +981,7 @@ static int bcm_qspi_exec_mem_op(struct spi_mem *mem,
if (mspi_read)
return bcm_qspi_mspi_exec_mem_op(spi, op);
- ret = bcm_qspi_bspi_set_mode(qspi, op, -1);
+ ret = bcm_qspi_bspi_set_mode(qspi, op, 0);
if (!ret)
ret = bcm_qspi_bspi_exec_mem_op(spi, op);
--
1.9.1
[PATCH v1 1/1] i2c: iproc: Fix i2c master read more than 63 bytes
Use SMBUS_MASTER_DATA_READ.MASTER_RD_STATUS bit to check for RX
FIFO empty condition because SMBUS_MASTER_FIFO_CONTROL.MASTER_RX_PKT_COUNT
is not updated for read >= 64 bytes. This fixes the issue when trying to
read from the I2C slave more than 63 bytes.
Fixes: c24b8d574b7c ("i2c: iproc: Extend I2C read up to 255 bytes")
Signed-off-by: Rayagonda Kokatanur
---
drivers/i2c/busses/i2c-bcm-iproc.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index 2c7f145..d7fd76b 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -392,16 +392,18 @@ static bool bcm_iproc_i2c_slave_isr(struct
bcm_iproc_i2c_dev *iproc_i2c,
static void bcm_iproc_i2c_read_valid_bytes(struct bcm_iproc_i2c_dev *iproc_i2c)
{
struct i2c_msg *msg = iproc_i2c->msg;
+ uint32_t val;
/* Read valid data from RX FIFO */
while (iproc_i2c->rx_bytes < msg->len) {
- if (!((iproc_i2c_rd_reg(iproc_i2c, M_FIFO_CTRL_OFFSET) >>
M_FIFO_RX_CNT_SHIFT)
- & M_FIFO_RX_CNT_MASK))
+ val = iproc_i2c_rd_reg(iproc_i2c, M_RX_OFFSET);
+
+ /* rx fifo empty */
+ if (!((val >> M_RX_STATUS_SHIFT) & M_RX_STATUS_MASK))
break;
msg->buf[iproc_i2c->rx_bytes] =
- (iproc_i2c_rd_reg(iproc_i2c, M_RX_OFFSET) >>
- M_RX_DATA_SHIFT) & M_RX_DATA_MASK;
+ (val >> M_RX_DATA_SHIFT) & M_RX_DATA_MASK;
iproc_i2c->rx_bytes++;
}
}
--
1.9.1
Re: [PATCH 1/1] i2c: iproc: Add multi byte read-write support for slave mode
Hi All,
Please review the patch and let me know your review comments if any.
Best regards,
Rayagonda
On Wed, May 15, 2019 at 9:58 PM Rayagonda Kokatanur
wrote:
>
> No change, it's just duplicate, please ignore first patch and review
> second patch.
>
> Best regards
> Rayagonda
>
>
>
> On Thu, May 9, 2019 at 9:58 PM Ray Jui wrote:
> >
> > Why is the email sent twice? What has changed?
> >
> > On 5/8/2019 9:21 PM, Rayagonda Kokatanur wrote:
> > > Add multiple byte read-write support for slave mode.
> > >
> > > Signed-off-by: Rayagonda Kokatanur
> > > Signed-off-by: Srinath Mannam
> > > ---
> > > drivers/i2c/busses/i2c-bcm-iproc.c | 117
> > > +
> > > 1 file changed, 53 insertions(+), 64 deletions(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
> > > b/drivers/i2c/busses/i2c-bcm-iproc.c
> > > index a845b8d..2c7f145 100644
> > > --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> > > +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> > > @@ -165,12 +165,6 @@ enum i2c_slave_read_status {
> > > I2C_SLAVE_RX_END,
> > > };
> > >
> > > -enum i2c_slave_xfer_dir {
> > > - I2C_SLAVE_DIR_READ = 0,
> > > - I2C_SLAVE_DIR_WRITE,
> > > - I2C_SLAVE_DIR_NONE,
> > > -};
> > > -
> > > enum bus_speed_index {
> > > I2C_SPD_100K = 0,
> > > I2C_SPD_400K,
> > > @@ -203,7 +197,6 @@ struct bcm_iproc_i2c_dev {
> > > struct i2c_msg *msg;
> > >
> > > struct i2c_client *slave;
> > > - enum i2c_slave_xfer_dir xfer_dir;
> > >
> > > /* bytes that have been transferred */
> > > unsigned int tx_bytes;
> > > @@ -219,7 +212,8 @@ struct bcm_iproc_i2c_dev {
> > > | BIT(IS_M_RX_THLD_SHIFT))
> > >
> > > #define ISR_MASK_SLAVE (BIT(IS_S_START_BUSY_SHIFT)\
> > > - | BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT))
> > > + | BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT)\
> > > + | BIT(IS_S_TX_UNDERRUN_SHIFT))
> > >
> > > static int bcm_iproc_i2c_reg_slave(struct i2c_client *slave);
> > > static int bcm_iproc_i2c_unreg_slave(struct i2c_client *slave);
> > > @@ -297,15 +291,11 @@ static void bcm_iproc_i2c_slave_init(
> > > /* clear all pending slave interrupts */
> > > iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET, ISR_MASK_SLAVE);
> > >
> > > - /* Enable interrupt register for any READ event */
> > > - val = BIT(IE_S_RD_EVENT_SHIFT);
> > > /* Enable interrupt register to indicate a valid byte in receive
> > > fifo */
> > > - val |= BIT(IE_S_RX_EVENT_SHIFT);
> > > + val = BIT(IE_S_RX_EVENT_SHIFT);
> > > /* Enable interrupt register for the Slave BUSY command */
> > > val |= BIT(IE_S_START_BUSY_SHIFT);
> > > iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, val);
> > > -
> > > - iproc_i2c->xfer_dir = I2C_SLAVE_DIR_NONE;
> > > }
> > >
> > > static void bcm_iproc_i2c_check_slave_status(
> > > @@ -314,8 +304,11 @@ static void bcm_iproc_i2c_check_slave_status(
> > > u32 val;
> > >
> > > val = iproc_i2c_rd_reg(iproc_i2c, S_CMD_OFFSET);
> > > - val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
> > > + /* status is valid only when START_BUSY is cleared after it was set
> > > */
> > > + if (val & BIT(S_CMD_START_BUSY_SHIFT))
> > > + return;
> > >
> > > + val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
> > > if (val == S_CMD_STATUS_TIMEOUT) {
> > > dev_err(iproc_i2c->device, "slave random stretch time
> > > timeout\n");
> > >
> > > @@ -327,70 +320,66 @@ static void bcm_iproc_i2c_check_slave_status(
> > > }
> > >
> > > static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
> > > - u32 status)
> > > + u32 status)
> > > {
> > > - u8 value;
> > > u32 val;
> > > - u32 rd_status;
> > > - u32 tmp;
> > > + u8 value, rx_status;
> > >
> > > - /* Start of transaction. check address and populate the direction */
> > > - if (iproc_i2
Re: [PATCH 1/1] i2c: iproc: Add multi byte read-write support for slave mode
No change, it's just duplicate, please ignore first patch and review
second patch.
Best regards
Rayagonda
On Thu, May 9, 2019 at 9:58 PM Ray Jui wrote:
>
> Why is the email sent twice? What has changed?
>
> On 5/8/2019 9:21 PM, Rayagonda Kokatanur wrote:
> > Add multiple byte read-write support for slave mode.
> >
> > Signed-off-by: Rayagonda Kokatanur
> > Signed-off-by: Srinath Mannam
> > ---
> > drivers/i2c/busses/i2c-bcm-iproc.c | 117
> > +
> > 1 file changed, 53 insertions(+), 64 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
> > b/drivers/i2c/busses/i2c-bcm-iproc.c
> > index a845b8d..2c7f145 100644
> > --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> > +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> > @@ -165,12 +165,6 @@ enum i2c_slave_read_status {
> > I2C_SLAVE_RX_END,
> > };
> >
> > -enum i2c_slave_xfer_dir {
> > - I2C_SLAVE_DIR_READ = 0,
> > - I2C_SLAVE_DIR_WRITE,
> > - I2C_SLAVE_DIR_NONE,
> > -};
> > -
> > enum bus_speed_index {
> > I2C_SPD_100K = 0,
> > I2C_SPD_400K,
> > @@ -203,7 +197,6 @@ struct bcm_iproc_i2c_dev {
> > struct i2c_msg *msg;
> >
> > struct i2c_client *slave;
> > - enum i2c_slave_xfer_dir xfer_dir;
> >
> > /* bytes that have been transferred */
> > unsigned int tx_bytes;
> > @@ -219,7 +212,8 @@ struct bcm_iproc_i2c_dev {
> > | BIT(IS_M_RX_THLD_SHIFT))
> >
> > #define ISR_MASK_SLAVE (BIT(IS_S_START_BUSY_SHIFT)\
> > - | BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT))
> > + | BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT)\
> > + | BIT(IS_S_TX_UNDERRUN_SHIFT))
> >
> > static int bcm_iproc_i2c_reg_slave(struct i2c_client *slave);
> > static int bcm_iproc_i2c_unreg_slave(struct i2c_client *slave);
> > @@ -297,15 +291,11 @@ static void bcm_iproc_i2c_slave_init(
> > /* clear all pending slave interrupts */
> > iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET, ISR_MASK_SLAVE);
> >
> > - /* Enable interrupt register for any READ event */
> > - val = BIT(IE_S_RD_EVENT_SHIFT);
> > /* Enable interrupt register to indicate a valid byte in receive fifo
> > */
> > - val |= BIT(IE_S_RX_EVENT_SHIFT);
> > + val = BIT(IE_S_RX_EVENT_SHIFT);
> > /* Enable interrupt register for the Slave BUSY command */
> > val |= BIT(IE_S_START_BUSY_SHIFT);
> > iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, val);
> > -
> > - iproc_i2c->xfer_dir = I2C_SLAVE_DIR_NONE;
> > }
> >
> > static void bcm_iproc_i2c_check_slave_status(
> > @@ -314,8 +304,11 @@ static void bcm_iproc_i2c_check_slave_status(
> > u32 val;
> >
> > val = iproc_i2c_rd_reg(iproc_i2c, S_CMD_OFFSET);
> > - val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
> > + /* status is valid only when START_BUSY is cleared after it was set */
> > + if (val & BIT(S_CMD_START_BUSY_SHIFT))
> > + return;
> >
> > + val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
> > if (val == S_CMD_STATUS_TIMEOUT) {
> > dev_err(iproc_i2c->device, "slave random stretch time
> > timeout\n");
> >
> > @@ -327,70 +320,66 @@ static void bcm_iproc_i2c_check_slave_status(
> > }
> >
> > static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
> > - u32 status)
> > + u32 status)
> > {
> > - u8 value;
> > u32 val;
> > - u32 rd_status;
> > - u32 tmp;
> > + u8 value, rx_status;
> >
> > - /* Start of transaction. check address and populate the direction */
> > - if (iproc_i2c->xfer_dir == I2C_SLAVE_DIR_NONE) {
> > - tmp = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
> > - rd_status = (tmp >> S_RX_STATUS_SHIFT) & S_RX_STATUS_MASK;
> > - /* This condition checks whether the request is a new request
> > */
> > - if (((rd_status == I2C_SLAVE_RX_START) &&
> > - (status & BIT(IS_S_RX_EVENT_SHIFT))) ||
> > - ((rd_status == I2C_SLAVE_RX_END) &&
> > - (status & BIT(IS_S_RD_EVENT_SHIFT {
> > -
> > - /* Last bit is W/R bit.
> > - * If 1 then its a read
[PATCH 1/1] i2c: iproc: Add multi byte read-write support for slave mode
Add multiple byte read-write support for slave mode.
Signed-off-by: Rayagonda Kokatanur
Signed-off-by: Srinath Mannam
---
drivers/i2c/busses/i2c-bcm-iproc.c | 117 +
1 file changed, 53 insertions(+), 64 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index a845b8d..2c7f145 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -165,12 +165,6 @@ enum i2c_slave_read_status {
I2C_SLAVE_RX_END,
};
-enum i2c_slave_xfer_dir {
- I2C_SLAVE_DIR_READ = 0,
- I2C_SLAVE_DIR_WRITE,
- I2C_SLAVE_DIR_NONE,
-};
-
enum bus_speed_index {
I2C_SPD_100K = 0,
I2C_SPD_400K,
@@ -203,7 +197,6 @@ struct bcm_iproc_i2c_dev {
struct i2c_msg *msg;
struct i2c_client *slave;
- enum i2c_slave_xfer_dir xfer_dir;
/* bytes that have been transferred */
unsigned int tx_bytes;
@@ -219,7 +212,8 @@ struct bcm_iproc_i2c_dev {
| BIT(IS_M_RX_THLD_SHIFT))
#define ISR_MASK_SLAVE (BIT(IS_S_START_BUSY_SHIFT)\
- | BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT))
+ | BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT)\
+ | BIT(IS_S_TX_UNDERRUN_SHIFT))
static int bcm_iproc_i2c_reg_slave(struct i2c_client *slave);
static int bcm_iproc_i2c_unreg_slave(struct i2c_client *slave);
@@ -297,15 +291,11 @@ static void bcm_iproc_i2c_slave_init(
/* clear all pending slave interrupts */
iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET, ISR_MASK_SLAVE);
- /* Enable interrupt register for any READ event */
- val = BIT(IE_S_RD_EVENT_SHIFT);
/* Enable interrupt register to indicate a valid byte in receive fifo */
- val |= BIT(IE_S_RX_EVENT_SHIFT);
+ val = BIT(IE_S_RX_EVENT_SHIFT);
/* Enable interrupt register for the Slave BUSY command */
val |= BIT(IE_S_START_BUSY_SHIFT);
iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, val);
-
- iproc_i2c->xfer_dir = I2C_SLAVE_DIR_NONE;
}
static void bcm_iproc_i2c_check_slave_status(
@@ -314,8 +304,11 @@ static void bcm_iproc_i2c_check_slave_status(
u32 val;
val = iproc_i2c_rd_reg(iproc_i2c, S_CMD_OFFSET);
- val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
+ /* status is valid only when START_BUSY is cleared after it was set */
+ if (val & BIT(S_CMD_START_BUSY_SHIFT))
+ return;
+ val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
if (val == S_CMD_STATUS_TIMEOUT) {
dev_err(iproc_i2c->device, "slave random stretch time
timeout\n");
@@ -327,70 +320,66 @@ static void bcm_iproc_i2c_check_slave_status(
}
static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
- u32 status)
+ u32 status)
{
- u8 value;
u32 val;
- u32 rd_status;
- u32 tmp;
+ u8 value, rx_status;
- /* Start of transaction. check address and populate the direction */
- if (iproc_i2c->xfer_dir == I2C_SLAVE_DIR_NONE) {
- tmp = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
- rd_status = (tmp >> S_RX_STATUS_SHIFT) & S_RX_STATUS_MASK;
- /* This condition checks whether the request is a new request */
- if (((rd_status == I2C_SLAVE_RX_START) &&
- (status & BIT(IS_S_RX_EVENT_SHIFT))) ||
- ((rd_status == I2C_SLAVE_RX_END) &&
- (status & BIT(IS_S_RD_EVENT_SHIFT {
-
- /* Last bit is W/R bit.
-* If 1 then its a read request(by master).
-*/
- iproc_i2c->xfer_dir = tmp & SLAVE_READ_WRITE_BIT_MASK;
- if (iproc_i2c->xfer_dir == I2C_SLAVE_DIR_WRITE)
- i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_READ_REQUESTED, &value);
- else
- i2c_slave_event(iproc_i2c->slave,
+ /* Slave RX byte receive */
+ if (status & BIT(IS_S_RX_EVENT_SHIFT)) {
+ val = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
+ rx_status = (val >> S_RX_STATUS_SHIFT) & S_RX_STATUS_MASK;
+ if (rx_status == I2C_SLAVE_RX_START) {
+ /* Start of SMBUS for Master write */
+ i2c_slave_event(iproc_i2c->slave,
I2C_SLAVE_WRITE_REQUESTED, &value);
- }
- }
- /* read request from master */
- if ((status & BIT(IS_S_RD_EVENT_SHIFT)) &&
- (iproc_i2c->xfer_dir == I2C_SLAVE_DIR_WRITE)) {
- i2c_slave_event(iproc_
[PATCH 1/1] i2c: iproc: Add multi byte read-write support for slave mode
Add multiple byte read-write support for slave mode.
Signed-off-by: Rayagonda Kokatanur
Signed-off-by: Srinath Mannam
---
drivers/i2c/busses/i2c-bcm-iproc.c | 117 +
1 file changed, 53 insertions(+), 64 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c
b/drivers/i2c/busses/i2c-bcm-iproc.c
index a845b8d..2c7f145 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -165,12 +165,6 @@ enum i2c_slave_read_status {
I2C_SLAVE_RX_END,
};
-enum i2c_slave_xfer_dir {
- I2C_SLAVE_DIR_READ = 0,
- I2C_SLAVE_DIR_WRITE,
- I2C_SLAVE_DIR_NONE,
-};
-
enum bus_speed_index {
I2C_SPD_100K = 0,
I2C_SPD_400K,
@@ -203,7 +197,6 @@ struct bcm_iproc_i2c_dev {
struct i2c_msg *msg;
struct i2c_client *slave;
- enum i2c_slave_xfer_dir xfer_dir;
/* bytes that have been transferred */
unsigned int tx_bytes;
@@ -219,7 +212,8 @@ struct bcm_iproc_i2c_dev {
| BIT(IS_M_RX_THLD_SHIFT))
#define ISR_MASK_SLAVE (BIT(IS_S_START_BUSY_SHIFT)\
- | BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT))
+ | BIT(IS_S_RX_EVENT_SHIFT) | BIT(IS_S_RD_EVENT_SHIFT)\
+ | BIT(IS_S_TX_UNDERRUN_SHIFT))
static int bcm_iproc_i2c_reg_slave(struct i2c_client *slave);
static int bcm_iproc_i2c_unreg_slave(struct i2c_client *slave);
@@ -297,15 +291,11 @@ static void bcm_iproc_i2c_slave_init(
/* clear all pending slave interrupts */
iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET, ISR_MASK_SLAVE);
- /* Enable interrupt register for any READ event */
- val = BIT(IE_S_RD_EVENT_SHIFT);
/* Enable interrupt register to indicate a valid byte in receive fifo */
- val |= BIT(IE_S_RX_EVENT_SHIFT);
+ val = BIT(IE_S_RX_EVENT_SHIFT);
/* Enable interrupt register for the Slave BUSY command */
val |= BIT(IE_S_START_BUSY_SHIFT);
iproc_i2c_wr_reg(iproc_i2c, IE_OFFSET, val);
-
- iproc_i2c->xfer_dir = I2C_SLAVE_DIR_NONE;
}
static void bcm_iproc_i2c_check_slave_status(
@@ -314,8 +304,11 @@ static void bcm_iproc_i2c_check_slave_status(
u32 val;
val = iproc_i2c_rd_reg(iproc_i2c, S_CMD_OFFSET);
- val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
+ /* status is valid only when START_BUSY is cleared after it was set */
+ if (val & BIT(S_CMD_START_BUSY_SHIFT))
+ return;
+ val = (val >> S_CMD_STATUS_SHIFT) & S_CMD_STATUS_MASK;
if (val == S_CMD_STATUS_TIMEOUT) {
dev_err(iproc_i2c->device, "slave random stretch time
timeout\n");
@@ -327,70 +320,66 @@ static void bcm_iproc_i2c_check_slave_status(
}
static bool bcm_iproc_i2c_slave_isr(struct bcm_iproc_i2c_dev *iproc_i2c,
- u32 status)
+ u32 status)
{
- u8 value;
u32 val;
- u32 rd_status;
- u32 tmp;
+ u8 value, rx_status;
- /* Start of transaction. check address and populate the direction */
- if (iproc_i2c->xfer_dir == I2C_SLAVE_DIR_NONE) {
- tmp = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
- rd_status = (tmp >> S_RX_STATUS_SHIFT) & S_RX_STATUS_MASK;
- /* This condition checks whether the request is a new request */
- if (((rd_status == I2C_SLAVE_RX_START) &&
- (status & BIT(IS_S_RX_EVENT_SHIFT))) ||
- ((rd_status == I2C_SLAVE_RX_END) &&
- (status & BIT(IS_S_RD_EVENT_SHIFT {
-
- /* Last bit is W/R bit.
-* If 1 then its a read request(by master).
-*/
- iproc_i2c->xfer_dir = tmp & SLAVE_READ_WRITE_BIT_MASK;
- if (iproc_i2c->xfer_dir == I2C_SLAVE_DIR_WRITE)
- i2c_slave_event(iproc_i2c->slave,
- I2C_SLAVE_READ_REQUESTED, &value);
- else
- i2c_slave_event(iproc_i2c->slave,
+ /* Slave RX byte receive */
+ if (status & BIT(IS_S_RX_EVENT_SHIFT)) {
+ val = iproc_i2c_rd_reg(iproc_i2c, S_RX_OFFSET);
+ rx_status = (val >> S_RX_STATUS_SHIFT) & S_RX_STATUS_MASK;
+ if (rx_status == I2C_SLAVE_RX_START) {
+ /* Start of SMBUS for Master write */
+ i2c_slave_event(iproc_i2c->slave,
I2C_SLAVE_WRITE_REQUESTED, &value);
- }
- }
- /* read request from master */
- if ((status & BIT(IS_S_RD_EVENT_SHIFT)) &&
- (iproc_i2c->xfer_dir == I2C_SLAVE_DIR_WRITE)) {
- i2c_slave_event(iproc_
Re: [PATCH v5 2/8] i2c: iproc: Add slave mode support
Hi Ray/Wolfram,
On Tue, Apr 2, 2019 at 3:03 AM Ray Jui wrote:
>
> Hi Wolfram/Rayagonda,
>
> On 3/27/2019 3:14 PM, Wolfram Sang wrote:
> >
> >> +static void bcm_iproc_i2c_slave_init(
> >> +struct bcm_iproc_i2c_dev *iproc_i2c, bool need_reset)
> >> +{
> >> +u32 val;
> >> +
> >> +if (need_reset) {
> >> +/* put controller in reset */
> >> +val = readl(iproc_i2c->base + CFG_OFFSET);
> >> +val |= BIT(CFG_RESET_SHIFT);
> >> +writel(val, iproc_i2c->base + CFG_OFFSET);
> >> +
> >> +/* wait 100 usec per spec */
> >> +udelay(100);
> >> +
> >> +/* bring controller out of reset */
> >> +val &= ~(BIT(CFG_RESET_SHIFT));
> >> +writel(val, iproc_i2c->base + CFG_OFFSET);
> >> +}
> >> +
> >> +/* flush TX/RX FIFOs */
> >> +val = (BIT(S_FIFO_RX_FLUSH_SHIFT) | BIT(S_FIFO_TX_FLUSH_SHIFT));
> >> +writel(val, iproc_i2c->base + S_FIFO_CTRL_OFFSET);
> >
> > Will flushing FIFOs work when a slave is register while a master
> > transfer is on-going at the same time?
> >
>
> Okay, as you pointed out in a subsequent email, this can't happen.
>
> >> +
> >> +/* RANDOM SLAVE STRETCH time - 20ms*/
> >
> > What is a "random stretch time"? 20ms sounds like a lot. Also, missing
> > space before comment terminator.
> >
>
> Rayagonda,
>
> Could you please help to comment on the choice of the 20 ms to allow
> clock stretch from the slave? In probably all cases, the slave should
> not need more than 1 ms? 20 ms does seem way too long as Wolfram pointed
> out.
In fact we are programming max slave stretch time ie 25ms, comment
should be correcting.
Its maximum time for slave to complete read/write operation, if slave
is done with read/write then clock will not be stretched further, it
will be released immediately.
Hence I feel no harm in programming max timeout value.
Also determining correct slave stretch is very subjective and depends
on slave type as well.
Best regards,
Rayagonda
>
> Will fix the missing space before comment terminator.
>
> >> @@ -224,22 +473,25 @@ static int bcm_iproc_i2c_init(struct
> >> bcm_iproc_i2c_dev *iproc_i2c)
> >>
> >> /* put controller in reset */
> >> val = readl(iproc_i2c->base + CFG_OFFSET);
> >> -val |= 1 << CFG_RESET_SHIFT;
> >> -val &= ~(1 << CFG_EN_SHIFT);
> >> +val |= BIT(CFG_RESET_SHIFT);
> >> +val &= ~(BIT(CFG_EN_SHIFT));
> >> writel(val, iproc_i2c->base + CFG_OFFSET);
> >>
> >> /* wait 100 usec per spec */
> >> udelay(100);
> >>
> >> /* bring controller out of reset */
> >> -val &= ~(1 << CFG_RESET_SHIFT);
> >> +val &= ~(BIT(CFG_RESET_SHIFT));
> >> writel(val, iproc_i2c->base + CFG_OFFSET);
> >>
> >> /* flush TX/RX FIFOs and set RX FIFO threshold to zero */
> >> -val = (1 << M_FIFO_RX_FLUSH_SHIFT) | (1 << M_FIFO_TX_FLUSH_SHIFT);
> >> +val = (BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT));
> >> writel(val, iproc_i2c->base + M_FIFO_CTRL_OFFSET);
> >> /* disable all interrupts */
> >> -writel(0, iproc_i2c->base + IE_OFFSET);
> >> +val = readl(iproc_i2c->base + IE_OFFSET);
> >> +val &= ~(IE_M_ALL_INTERRUPT_MASK <<
> >> +IE_M_ALL_INTERRUPT_SHIFT);
> >> +writel(val, iproc_i2c->base + IE_OFFSET);
> >
> > This block looks unrelated, but I won't be too strict here...
> >
> >> +case M_CMD_STATUS_FIFO_UNDERRUN:
> >> +dev_dbg(iproc_i2c->device, "FIFO under-run\n");
> >> +return -ENXIO;
> >> +
> >> +case M_CMD_STATUS_RX_FIFO_FULL:
> >> +dev_dbg(iproc_i2c->device, "Master Rx FIFO full > 10ms\n");
> >> +return -ETIMEDOUT;
> >> +
> >
> > ... however, this looks really unrelated to me. This is about master
> > transmission, or?
>
> This should be submitted in a separate commit. Will do that in the next
> iteration of patch series.
>
> >
> > Rest looks OK.
> >
>
> Thanks,
>
> Ray
