| Message ID | 05e89ea8549634e750d783360590f2802954437d.camel@kernel.crashing.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [linux,dev-4.13,1/6] gpio/aspeed: Set output latch before changing direction | expand |
Reviewed-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> On 5/7/18 8:46 PM, Benjamin Herrenschmidt wrote: > FSI_GPIO_DPOLL_CLOCKS is the number of clocks before sending > a DPOLL command after receiving a BUSY status. It should be > at least tSendDelay (16 clocks). > > According to comments in the code, it needs to also be at least > 21 clocks due to HW issues. > > It's currently 100 clocks which impacts performances negatively > in some cases. Reduces it in half to 50 clocks which seems to > still be solid. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > drivers/fsi/fsi-master-gpio.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c > index 029b0a5b6d89..bd2b2cbd5eb5 100644 > --- a/drivers/fsi/fsi-master-gpio.c > +++ b/drivers/fsi/fsi-master-gpio.c > @@ -29,7 +29,8 @@ > #define FSI_GPIO_CMD_TERM 0x3f > #define FSI_GPIO_CMD_ABS_AR 0x4 > > -#define FSI_GPIO_DPOLL_CLOCKS 100 /* < 21 will cause slave to hang */ > + > +#define FSI_GPIO_DPOLL_CLOCKS 50 /* < 21 will cause slave to hang */ > > /* Bus errors */ > #define FSI_GPIO_ERR_BUSY 1 /* Slave stuck in busy state */ > @@ -43,7 +44,7 @@ > #define FSI_GPIO_RESP_ACK 0 > #define FSI_GPIO_RESP_ACKD 4 > > -#define FSI_GPIO_MAX_BUSY 100 > +#define FSI_GPIO_MAX_BUSY 200 > #define FSI_GPIO_MTOE_COUNT 1000 > #define FSI_GPIO_DRAIN_BITS 20 > #define FSI_GPIO_CRC_SIZE 4 >
On Wed, 9 May 2018, at 05:30, Christopher Bostic wrote: > Reviewed-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> > > > On 5/7/18 8:46 PM, Benjamin Herrenschmidt wrote: > > FSI_GPIO_DPOLL_CLOCKS is the number of clocks before sending > > a DPOLL command after receiving a BUSY status. It should be > > at least tSendDelay (16 clocks). > > > > According to comments in the code, it needs to also be at least > > 21 clocks due to HW issues. > > > > It's currently 100 clocks which impacts performances negatively > > in some cases. Reduces it in half to 50 clocks which seems to > > still be solid. Out of curiosity, was there any science to 50 beyond the (16/21 cycle) reasoning above? Was there instability at lower values? > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > --- > > drivers/fsi/fsi-master-gpio.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c > > index 029b0a5b6d89..bd2b2cbd5eb5 100644 > > --- a/drivers/fsi/fsi-master-gpio.c > > +++ b/drivers/fsi/fsi-master-gpio.c > > @@ -29,7 +29,8 @@ > > #define FSI_GPIO_CMD_TERM 0x3f > > #define FSI_GPIO_CMD_ABS_AR 0x4 > > > > -#define FSI_GPIO_DPOLL_CLOCKS 100 /* < 21 will cause slave to hang */ > > + > > +#define FSI_GPIO_DPOLL_CLOCKS 50 /* < 21 will cause slave to hang */ > > > > /* Bus errors */ > > #define FSI_GPIO_ERR_BUSY 1 /* Slave stuck in busy state */ > > @@ -43,7 +44,7 @@ > > #define FSI_GPIO_RESP_ACK 0 > > #define FSI_GPIO_RESP_ACKD 4 > > > > -#define FSI_GPIO_MAX_BUSY 100 > > +#define FSI_GPIO_MAX_BUSY 200 > > #define FSI_GPIO_MTOE_COUNT 1000 > > #define FSI_GPIO_DRAIN_BITS 20 > > #define FSI_GPIO_CRC_SIZE 4 > > >
On Thu, 2018-05-10 at 12:36 +0930, Andrew Jeffery wrote: > On Wed, 9 May 2018, at 05:30, Christopher Bostic wrote: > > Reviewed-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> > > > > > > On 5/7/18 8:46 PM, Benjamin Herrenschmidt wrote: > > > FSI_GPIO_DPOLL_CLOCKS is the number of clocks before sending > > > a DPOLL command after receiving a BUSY status. It should be > > > at least tSendDelay (16 clocks). > > > > > > According to comments in the code, it needs to also be at least > > > 21 clocks due to HW issues. > > > > > > It's currently 100 clocks which impacts performances negatively > > > in some cases. Reduces it in half to 50 clocks which seems to > > > still be solid. > > Out of curiosity, was there any science to 50 beyond the (16/21 > cycle) reasoning above? Was there instability at lower values? No. The "science" aka spec would need it to be tSendDelay, which is 16 cycles. However there's a comment that says that there are issues below 21. I decided to keep *some* paranoia and cut in half to 50 instead of 100. > > > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > --- > > > drivers/fsi/fsi-master-gpio.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c > > > index 029b0a5b6d89..bd2b2cbd5eb5 100644 > > > --- a/drivers/fsi/fsi-master-gpio.c > > > +++ b/drivers/fsi/fsi-master-gpio.c > > > @@ -29,7 +29,8 @@ > > > #define FSI_GPIO_CMD_TERM 0x3f > > > #define FSI_GPIO_CMD_ABS_AR 0x4 > > > > > > -#define FSI_GPIO_DPOLL_CLOCKS 100 /* < 21 will cause slave to hang */ > > > + > > > +#define FSI_GPIO_DPOLL_CLOCKS 50 /* < 21 will cause slave to hang */ > > > > > > /* Bus errors */ > > > #define FSI_GPIO_ERR_BUSY 1 /* Slave stuck in busy state */ > > > @@ -43,7 +44,7 @@ > > > #define FSI_GPIO_RESP_ACK 0 > > > #define FSI_GPIO_RESP_ACKD 4 > > > > > > -#define FSI_GPIO_MAX_BUSY 100 > > > +#define FSI_GPIO_MAX_BUSY 200 > > > #define FSI_GPIO_MTOE_COUNT 1000 > > > #define FSI_GPIO_DRAIN_BITS 20 > > > #define FSI_GPIO_CRC_SIZE 4 > > >
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c index 029b0a5b6d89..bd2b2cbd5eb5 100644 --- a/drivers/fsi/fsi-master-gpio.c +++ b/drivers/fsi/fsi-master-gpio.c @@ -29,7 +29,8 @@ #define FSI_GPIO_CMD_TERM 0x3f #define FSI_GPIO_CMD_ABS_AR 0x4 -#define FSI_GPIO_DPOLL_CLOCKS 100 /* < 21 will cause slave to hang */ + +#define FSI_GPIO_DPOLL_CLOCKS 50 /* < 21 will cause slave to hang */ /* Bus errors */ #define FSI_GPIO_ERR_BUSY 1 /* Slave stuck in busy state */ @@ -43,7 +44,7 @@ #define FSI_GPIO_RESP_ACK 0 #define FSI_GPIO_RESP_ACKD 4 -#define FSI_GPIO_MAX_BUSY 100 +#define FSI_GPIO_MAX_BUSY 200 #define FSI_GPIO_MTOE_COUNT 1000 #define FSI_GPIO_DRAIN_BITS 20 #define FSI_GPIO_CRC_SIZE 4
FSI_GPIO_DPOLL_CLOCKS is the number of clocks before sending a DPOLL command after receiving a BUSY status. It should be at least tSendDelay (16 clocks). According to comments in the code, it needs to also be at least 21 clocks due to HW issues. It's currently 100 clocks which impacts performances negatively in some cases. Reduces it in half to 50 clocks which seems to still be solid. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- drivers/fsi/fsi-master-gpio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)