* [PATCH 0/5] i2c: cadence: Small cleanups
@ 2023-01-07 21:18 Lars-Peter Clausen
2023-01-07 21:18 ` [PATCH 1/5] i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define Lars-Peter Clausen
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Lars-Peter Clausen @ 2023-01-07 21:18 UTC (permalink / raw)
To: Wolfram Sang
Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c, Lars-Peter Clausen
This series has a set of small cleanups for the Cadence I2C driver. The
changes are mostly cosmetical and there is no change in behavior of the
driver.
Lars-Peter Clausen (5):
i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define
i2c: cadence: Remove `irq` field from driver state struct
i2c: cadence: Remove redundant expression in if clause
i2c: cadence: Remove always false ternary operator
i2c: cadence: Remove unnecessary register reads
drivers/i2c/busses/i2c-cadence.c | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/5] i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define 2023-01-07 21:18 [PATCH 0/5] i2c: cadence: Small cleanups Lars-Peter Clausen @ 2023-01-07 21:18 ` Lars-Peter Clausen 2023-01-16 14:37 ` Michal Simek 2023-01-20 8:56 ` Wolfram Sang 2023-01-07 21:18 ` [PATCH 2/5] i2c: cadence: Remove `irq` field from driver state struct Lars-Peter Clausen ` (4 subsequent siblings) 5 siblings, 2 replies; 21+ messages in thread From: Lars-Peter Clausen @ 2023-01-07 21:18 UTC (permalink / raw) To: Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c, Lars-Peter Clausen The CDNS_I2C_DATA_INTR_DEPTH is not used in the Cadence I2C driver. Remove it. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- drivers/i2c/busses/i2c-cadence.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c index f58943cb1341..71ea658f4bd3 100644 --- a/drivers/i2c/busses/i2c-cadence.c +++ b/drivers/i2c/busses/i2c-cadence.c @@ -115,8 +115,6 @@ #define CNDS_I2C_PM_TIMEOUT 1000 /* ms */ #define CDNS_I2C_FIFO_DEPTH 16 -/* FIFO depth at which the DATA interrupt occurs */ -#define CDNS_I2C_DATA_INTR_DEPTH (CDNS_I2C_FIFO_DEPTH - 2) #define CDNS_I2C_MAX_TRANSFER_SIZE 255 /* Transfer size in multiples of data interrupt depth */ #define CDNS_I2C_TRANSFER_SIZE (CDNS_I2C_MAX_TRANSFER_SIZE - 3) -- 2.30.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define 2023-01-07 21:18 ` [PATCH 1/5] i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define Lars-Peter Clausen @ 2023-01-16 14:37 ` Michal Simek 2023-01-20 8:56 ` Wolfram Sang 1 sibling, 0 replies; 21+ messages in thread From: Michal Simek @ 2023-01-16 14:37 UTC (permalink / raw) To: Lars-Peter Clausen, Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c On 1/7/23 22:18, Lars-Peter Clausen wrote: > > The CDNS_I2C_DATA_INTR_DEPTH is not used in the Cadence I2C driver. Remove > it. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > drivers/i2c/busses/i2c-cadence.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c > index f58943cb1341..71ea658f4bd3 100644 > --- a/drivers/i2c/busses/i2c-cadence.c > +++ b/drivers/i2c/busses/i2c-cadence.c > @@ -115,8 +115,6 @@ > #define CNDS_I2C_PM_TIMEOUT 1000 /* ms */ > > #define CDNS_I2C_FIFO_DEPTH 16 > -/* FIFO depth at which the DATA interrupt occurs */ > -#define CDNS_I2C_DATA_INTR_DEPTH (CDNS_I2C_FIFO_DEPTH - 2) > #define CDNS_I2C_MAX_TRANSFER_SIZE 255 > /* Transfer size in multiples of data interrupt depth */ > #define CDNS_I2C_TRANSFER_SIZE (CDNS_I2C_MAX_TRANSFER_SIZE - 3) > -- > 2.30.2 > Reviewed-by: Michal Simek <michal.simek@amd.com> Thanks, Michal ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define 2023-01-07 21:18 ` [PATCH 1/5] i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define Lars-Peter Clausen 2023-01-16 14:37 ` Michal Simek @ 2023-01-20 8:56 ` Wolfram Sang 1 sibling, 0 replies; 21+ messages in thread From: Wolfram Sang @ 2023-01-20 8:56 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c [-- Attachment #1: Type: text/plain, Size: 247 bytes --] On Sat, Jan 07, 2023 at 01:18:10PM -0800, Lars-Peter Clausen wrote: > The CDNS_I2C_DATA_INTR_DEPTH is not used in the Cadence I2C driver. Remove > it. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] i2c: cadence: Remove `irq` field from driver state struct 2023-01-07 21:18 [PATCH 0/5] i2c: cadence: Small cleanups Lars-Peter Clausen 2023-01-07 21:18 ` [PATCH 1/5] i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define Lars-Peter Clausen @ 2023-01-07 21:18 ` Lars-Peter Clausen 2023-01-16 14:39 ` Michal Simek 2023-01-20 8:56 ` Wolfram Sang 2023-01-07 21:18 ` [PATCH 3/5] i2c: cadence: Remove redundant expression in if clause Lars-Peter Clausen ` (3 subsequent siblings) 5 siblings, 2 replies; 21+ messages in thread From: Lars-Peter Clausen @ 2023-01-07 21:18 UTC (permalink / raw) To: Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c, Lars-Peter Clausen The irq field of the driver state struct is only used in the probe function. There is no need to keep it around. Remove it from the state struct and use a on-stack variable instead. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- drivers/i2c/busses/i2c-cadence.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c index 71ea658f4bd3..e2a4cb694cfb 100644 --- a/drivers/i2c/busses/i2c-cadence.c +++ b/drivers/i2c/busses/i2c-cadence.c @@ -173,7 +173,6 @@ enum cdns_i2c_slave_state { * @send_count: Number of bytes still expected to send * @recv_count: Number of bytes still expected to receive * @curr_recv_count: Number of bytes to be received in current transfer - * @irq: IRQ number * @input_clk: Input clock to I2C controller * @i2c_clk: Maximum I2C clock speed * @bus_hold_flag: Flag used in repeated start for clearing HOLD bit @@ -198,7 +197,6 @@ struct cdns_i2c { unsigned int send_count; unsigned int recv_count; unsigned int curr_recv_count; - int irq; unsigned long input_clk; unsigned int i2c_clk; unsigned int bus_hold_flag; @@ -1244,7 +1242,7 @@ static int cdns_i2c_probe(struct platform_device *pdev) { struct resource *r_mem; struct cdns_i2c *id; - int ret; + int ret, irq; const struct of_device_id *match; id = devm_kzalloc(&pdev->dev, sizeof(*id), GFP_KERNEL); @@ -1275,10 +1273,9 @@ static int cdns_i2c_probe(struct platform_device *pdev) if (IS_ERR(id->membase)) return PTR_ERR(id->membase); - ret = platform_get_irq(pdev, 0); - if (ret < 0) - return ret; - id->irq = ret; + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return irq; id->adap.owner = THIS_MODULE; id->adap.dev.of_node = pdev->dev.of_node; @@ -1329,10 +1326,10 @@ static int cdns_i2c_probe(struct platform_device *pdev) goto err_clk_dis; } - ret = devm_request_irq(&pdev->dev, id->irq, cdns_i2c_isr, 0, + ret = devm_request_irq(&pdev->dev, irq, cdns_i2c_isr, 0, DRIVER_NAME, id); if (ret) { - dev_err(&pdev->dev, "cannot get irq %d\n", id->irq); + dev_err(&pdev->dev, "cannot get irq %d\n", irq); goto err_clk_dis; } cdns_i2c_init(id); @@ -1342,7 +1339,7 @@ static int cdns_i2c_probe(struct platform_device *pdev) goto err_clk_dis; dev_info(&pdev->dev, "%u kHz mmio %08lx irq %d\n", - id->i2c_clk / 1000, (unsigned long)r_mem->start, id->irq); + id->i2c_clk / 1000, (unsigned long)r_mem->start, irq); return 0; -- 2.30.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] i2c: cadence: Remove `irq` field from driver state struct 2023-01-07 21:18 ` [PATCH 2/5] i2c: cadence: Remove `irq` field from driver state struct Lars-Peter Clausen @ 2023-01-16 14:39 ` Michal Simek 2023-01-20 8:56 ` Wolfram Sang 1 sibling, 0 replies; 21+ messages in thread From: Michal Simek @ 2023-01-16 14:39 UTC (permalink / raw) To: Lars-Peter Clausen, Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c On 1/7/23 22:18, Lars-Peter Clausen wrote: > > The irq field of the driver state struct is only used in the probe > function. There is no need to keep it around. Remove it from the state > struct and use a on-stack variable instead. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > drivers/i2c/busses/i2c-cadence.c | 17 +++++++---------- > 1 file changed, 7 insertions(+), 10 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c > index 71ea658f4bd3..e2a4cb694cfb 100644 > --- a/drivers/i2c/busses/i2c-cadence.c > +++ b/drivers/i2c/busses/i2c-cadence.c > @@ -173,7 +173,6 @@ enum cdns_i2c_slave_state { > * @send_count: Number of bytes still expected to send > * @recv_count: Number of bytes still expected to receive > * @curr_recv_count: Number of bytes to be received in current transfer > - * @irq: IRQ number > * @input_clk: Input clock to I2C controller > * @i2c_clk: Maximum I2C clock speed > * @bus_hold_flag: Flag used in repeated start for clearing HOLD bit > @@ -198,7 +197,6 @@ struct cdns_i2c { > unsigned int send_count; > unsigned int recv_count; > unsigned int curr_recv_count; > - int irq; > unsigned long input_clk; > unsigned int i2c_clk; > unsigned int bus_hold_flag; > @@ -1244,7 +1242,7 @@ static int cdns_i2c_probe(struct platform_device *pdev) > { > struct resource *r_mem; > struct cdns_i2c *id; > - int ret; > + int ret, irq; > const struct of_device_id *match; > > id = devm_kzalloc(&pdev->dev, sizeof(*id), GFP_KERNEL); > @@ -1275,10 +1273,9 @@ static int cdns_i2c_probe(struct platform_device *pdev) > if (IS_ERR(id->membase)) > return PTR_ERR(id->membase); > > - ret = platform_get_irq(pdev, 0); > - if (ret < 0) > - return ret; > - id->irq = ret; > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > > id->adap.owner = THIS_MODULE; > id->adap.dev.of_node = pdev->dev.of_node; > @@ -1329,10 +1326,10 @@ static int cdns_i2c_probe(struct platform_device *pdev) > goto err_clk_dis; > } > > - ret = devm_request_irq(&pdev->dev, id->irq, cdns_i2c_isr, 0, > + ret = devm_request_irq(&pdev->dev, irq, cdns_i2c_isr, 0, > DRIVER_NAME, id); > if (ret) { > - dev_err(&pdev->dev, "cannot get irq %d\n", id->irq); > + dev_err(&pdev->dev, "cannot get irq %d\n", irq); > goto err_clk_dis; > } > cdns_i2c_init(id); > @@ -1342,7 +1339,7 @@ static int cdns_i2c_probe(struct platform_device *pdev) > goto err_clk_dis; > > dev_info(&pdev->dev, "%u kHz mmio %08lx irq %d\n", > - id->i2c_clk / 1000, (unsigned long)r_mem->start, id->irq); > + id->i2c_clk / 1000, (unsigned long)r_mem->start, irq); > > return 0; > > -- > 2.30.2 > Reviewed-by: Michal Simek <michal.simek@amd.com> Thanks, Michal ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] i2c: cadence: Remove `irq` field from driver state struct 2023-01-07 21:18 ` [PATCH 2/5] i2c: cadence: Remove `irq` field from driver state struct Lars-Peter Clausen 2023-01-16 14:39 ` Michal Simek @ 2023-01-20 8:56 ` Wolfram Sang 1 sibling, 0 replies; 21+ messages in thread From: Wolfram Sang @ 2023-01-20 8:56 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c [-- Attachment #1: Type: text/plain, Size: 353 bytes --] On Sat, Jan 07, 2023 at 01:18:11PM -0800, Lars-Peter Clausen wrote: > The irq field of the driver state struct is only used in the probe > function. There is no need to keep it around. Remove it from the state > struct and use a on-stack variable instead. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/5] i2c: cadence: Remove redundant expression in if clause 2023-01-07 21:18 [PATCH 0/5] i2c: cadence: Small cleanups Lars-Peter Clausen 2023-01-07 21:18 ` [PATCH 1/5] i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define Lars-Peter Clausen 2023-01-07 21:18 ` [PATCH 2/5] i2c: cadence: Remove `irq` field from driver state struct Lars-Peter Clausen @ 2023-01-07 21:18 ` Lars-Peter Clausen 2023-01-16 14:44 ` Michal Simek 2023-01-20 8:56 ` Wolfram Sang 2023-01-07 21:18 ` [PATCH 4/5] i2c: cadence: Remove always false ternary operator Lars-Peter Clausen ` (2 subsequent siblings) 5 siblings, 2 replies; 21+ messages in thread From: Lars-Peter Clausen @ 2023-01-07 21:18 UTC (permalink / raw) To: Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c, Lars-Peter Clausen In the mrecv() function the Cadence I2C driver has the following expression in an if clause. ((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) && (id->recv_count <= CDNS_I2C_FIFO_DEPTH)) Earlier in the same function when I2C_M_RECV_LEN is set the recv_count is initialized to a value that is larger than CDNS_I2C_FIFO_DEPTH. This means if the first expression is false the second expression is also false. Checking the first expression is thus redundant and can be removed. This slightly simplifies the logic. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- drivers/i2c/busses/i2c-cadence.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c index e2a4cb694cfb..b5d22e7282c2 100644 --- a/drivers/i2c/busses/i2c-cadence.c +++ b/drivers/i2c/busses/i2c-cadence.c @@ -612,9 +612,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) } /* Determine hold_clear based on number of bytes to receive and hold flag */ - if (!id->bus_hold_flag && - ((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) && - (id->recv_count <= CDNS_I2C_FIFO_DEPTH)) { + if (!id->bus_hold_flag && id->recv_count <= CDNS_I2C_FIFO_DEPTH) { if (cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & CDNS_I2C_CR_HOLD) { hold_clear = true; if (id->quirks & CDNS_I2C_BROKEN_HOLD_BIT) -- 2.30.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] i2c: cadence: Remove redundant expression in if clause 2023-01-07 21:18 ` [PATCH 3/5] i2c: cadence: Remove redundant expression in if clause Lars-Peter Clausen @ 2023-01-16 14:44 ` Michal Simek 2023-01-20 8:56 ` Wolfram Sang 1 sibling, 0 replies; 21+ messages in thread From: Michal Simek @ 2023-01-16 14:44 UTC (permalink / raw) To: Lars-Peter Clausen, Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c On 1/7/23 22:18, Lars-Peter Clausen wrote: > > > In the mrecv() function the Cadence I2C driver has the following expression > in an if clause. > > ((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) && > (id->recv_count <= CDNS_I2C_FIFO_DEPTH)) > > Earlier in the same function when I2C_M_RECV_LEN is set the recv_count is > initialized to a value that is larger than CDNS_I2C_FIFO_DEPTH. This means > if the first expression is false the second expression is also false. > Checking the first expression is thus redundant and can be removed. > > This slightly simplifies the logic. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > drivers/i2c/busses/i2c-cadence.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c > index e2a4cb694cfb..b5d22e7282c2 100644 > --- a/drivers/i2c/busses/i2c-cadence.c > +++ b/drivers/i2c/busses/i2c-cadence.c > @@ -612,9 +612,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) > } > > /* Determine hold_clear based on number of bytes to receive and hold flag */ > - if (!id->bus_hold_flag && > - ((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) && > - (id->recv_count <= CDNS_I2C_FIFO_DEPTH)) { > + if (!id->bus_hold_flag && id->recv_count <= CDNS_I2C_FIFO_DEPTH) { > if (cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & CDNS_I2C_CR_HOLD) { > hold_clear = true; > if (id->quirks & CDNS_I2C_BROKEN_HOLD_BIT) > -- > 2.30.2 > Reviewed-by: Michal Simek <michal.simek@amd.com> Thanks, Michal ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] i2c: cadence: Remove redundant expression in if clause 2023-01-07 21:18 ` [PATCH 3/5] i2c: cadence: Remove redundant expression in if clause Lars-Peter Clausen 2023-01-16 14:44 ` Michal Simek @ 2023-01-20 8:56 ` Wolfram Sang 1 sibling, 0 replies; 21+ messages in thread From: Wolfram Sang @ 2023-01-20 8:56 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c [-- Attachment #1: Type: text/plain, Size: 720 bytes --] On Sat, Jan 07, 2023 at 01:18:12PM -0800, Lars-Peter Clausen wrote: > In the mrecv() function the Cadence I2C driver has the following expression > in an if clause. > > ((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) && > (id->recv_count <= CDNS_I2C_FIFO_DEPTH)) > > Earlier in the same function when I2C_M_RECV_LEN is set the recv_count is > initialized to a value that is larger than CDNS_I2C_FIFO_DEPTH. This means > if the first expression is false the second expression is also false. > Checking the first expression is thus redundant and can be removed. > > This slightly simplifies the logic. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/5] i2c: cadence: Remove always false ternary operator 2023-01-07 21:18 [PATCH 0/5] i2c: cadence: Small cleanups Lars-Peter Clausen ` (2 preceding siblings ...) 2023-01-07 21:18 ` [PATCH 3/5] i2c: cadence: Remove redundant expression in if clause Lars-Peter Clausen @ 2023-01-07 21:18 ` Lars-Peter Clausen 2023-01-16 15:00 ` Michal Simek ` (2 more replies) 2023-01-07 21:18 ` [PATCH 5/5] i2c: cadence: Remove unnecessary register reads Lars-Peter Clausen [not found] ` <BY5PR12MB4902E8FC89477C6A1038F15281A09@BY5PR12MB4902.namprd12.prod.outlook.com> 5 siblings, 3 replies; 21+ messages in thread From: Lars-Peter Clausen @ 2023-01-07 21:18 UTC (permalink / raw) To: Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c, Lars-Peter Clausen When selecting the clock dividers the Cadence I2C driver skips settings where the resulting I2C bus frequency is larger than the requested frequency. If the resulting frequency is lower it will compute the error to actual frequency. When calculating the difference it also handles the case where the resulting frequency is larger. Since the resulting frequency is always smaller or equal the computation of the error can be simplified by only considering this case. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- drivers/i2c/busses/i2c-cadence.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c index b5d22e7282c2..bec50bfe7aad 100644 --- a/drivers/i2c/busses/i2c-cadence.c +++ b/drivers/i2c/busses/i2c-cadence.c @@ -1030,8 +1030,7 @@ static int cdns_i2c_calc_divs(unsigned long *f, unsigned long input_clk, if (actual_fscl > fscl) continue; - current_error = ((actual_fscl > fscl) ? (actual_fscl - fscl) : - (fscl - actual_fscl)); + current_error = fscl - actual_fscl; if (last_error > current_error) { calc_div_a = div_a; -- 2.30.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] i2c: cadence: Remove always false ternary operator 2023-01-07 21:18 ` [PATCH 4/5] i2c: cadence: Remove always false ternary operator Lars-Peter Clausen @ 2023-01-16 15:00 ` Michal Simek 2023-02-27 10:46 ` Michal Simek 2023-03-16 19:29 ` Wolfram Sang 2 siblings, 0 replies; 21+ messages in thread From: Michal Simek @ 2023-01-16 15:00 UTC (permalink / raw) To: Lars-Peter Clausen, Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c On 1/7/23 22:18, Lars-Peter Clausen wrote: > > When selecting the clock dividers the Cadence I2C driver skips settings > where the resulting I2C bus frequency is larger than the requested > frequency. > > If the resulting frequency is lower it will compute the error to actual > frequency. When calculating the difference it also handles the case where > the resulting frequency is larger. > > Since the resulting frequency is always smaller or equal the computation of > the error can be simplified by only considering this case. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > drivers/i2c/busses/i2c-cadence.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c > index b5d22e7282c2..bec50bfe7aad 100644 > --- a/drivers/i2c/busses/i2c-cadence.c > +++ b/drivers/i2c/busses/i2c-cadence.c > @@ -1030,8 +1030,7 @@ static int cdns_i2c_calc_divs(unsigned long *f, unsigned long input_clk, > if (actual_fscl > fscl) > continue; > > - current_error = ((actual_fscl > fscl) ? (actual_fscl - fscl) : > - (fscl - actual_fscl)); > + current_error = fscl - actual_fscl; > > if (last_error > current_error) { > calc_div_a = div_a; > -- > 2.30.2 > Shubhrajyoti: Please check this patch. Thanks, Michal ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] i2c: cadence: Remove always false ternary operator 2023-01-07 21:18 ` [PATCH 4/5] i2c: cadence: Remove always false ternary operator Lars-Peter Clausen 2023-01-16 15:00 ` Michal Simek @ 2023-02-27 10:46 ` Michal Simek 2023-03-16 19:29 ` Wolfram Sang 2 siblings, 0 replies; 21+ messages in thread From: Michal Simek @ 2023-02-27 10:46 UTC (permalink / raw) To: Lars-Peter Clausen, Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c On 1/7/23 22:18, Lars-Peter Clausen wrote: > CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email. > > > When selecting the clock dividers the Cadence I2C driver skips settings > where the resulting I2C bus frequency is larger than the requested > frequency. > > If the resulting frequency is lower it will compute the error to actual > frequency. When calculating the difference it also handles the case where > the resulting frequency is larger. > > Since the resulting frequency is always smaller or equal the computation of > the error can be simplified by only considering this case. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > drivers/i2c/busses/i2c-cadence.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c > index b5d22e7282c2..bec50bfe7aad 100644 > --- a/drivers/i2c/busses/i2c-cadence.c > +++ b/drivers/i2c/busses/i2c-cadence.c > @@ -1030,8 +1030,7 @@ static int cdns_i2c_calc_divs(unsigned long *f, unsigned long input_clk, > if (actual_fscl > fscl) > continue; > > - current_error = ((actual_fscl > fscl) ? (actual_fscl - fscl) : > - (fscl - actual_fscl)); > + current_error = fscl - actual_fscl; > > if (last_error > current_error) { > calc_div_a = div_a; > -- > 2.30.2 > Mani has tested it that's why Acked-by: Michal Simek <michal.simek@amd.com> Thanks, Michal ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] i2c: cadence: Remove always false ternary operator 2023-01-07 21:18 ` [PATCH 4/5] i2c: cadence: Remove always false ternary operator Lars-Peter Clausen 2023-01-16 15:00 ` Michal Simek 2023-02-27 10:46 ` Michal Simek @ 2023-03-16 19:29 ` Wolfram Sang 2 siblings, 0 replies; 21+ messages in thread From: Wolfram Sang @ 2023-03-16 19:29 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c [-- Attachment #1: Type: text/plain, Size: 660 bytes --] On Sat, Jan 07, 2023 at 01:18:13PM -0800, Lars-Peter Clausen wrote: > When selecting the clock dividers the Cadence I2C driver skips settings > where the resulting I2C bus frequency is larger than the requested > frequency. > > If the resulting frequency is lower it will compute the error to actual > frequency. When calculating the difference it also handles the case where > the resulting frequency is larger. > > Since the resulting frequency is always smaller or equal the computation of > the error can be simplified by only considering this case. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/5] i2c: cadence: Remove unnecessary register reads 2023-01-07 21:18 [PATCH 0/5] i2c: cadence: Small cleanups Lars-Peter Clausen ` (3 preceding siblings ...) 2023-01-07 21:18 ` [PATCH 4/5] i2c: cadence: Remove always false ternary operator Lars-Peter Clausen @ 2023-01-07 21:18 ` Lars-Peter Clausen 2023-01-16 14:58 ` Michal Simek 2023-03-16 19:29 ` Wolfram Sang [not found] ` <BY5PR12MB4902E8FC89477C6A1038F15281A09@BY5PR12MB4902.namprd12.prod.outlook.com> 5 siblings, 2 replies; 21+ messages in thread From: Lars-Peter Clausen @ 2023-01-07 21:18 UTC (permalink / raw) To: Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c, Lars-Peter Clausen In the `cdns_i2c_mrecv()` function the CTRL register of the Cadence I2C controller is written and read back multiple times. The register value does not change on its own. So it is possible to remember the just written value instead of reading it back from the hardware. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- drivers/i2c/busses/i2c-cadence.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c index bec50bfe7aad..93c6d0822468 100644 --- a/drivers/i2c/busses/i2c-cadence.c +++ b/drivers/i2c/busses/i2c-cadence.c @@ -613,7 +613,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) /* Determine hold_clear based on number of bytes to receive and hold flag */ if (!id->bus_hold_flag && id->recv_count <= CDNS_I2C_FIFO_DEPTH) { - if (cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & CDNS_I2C_CR_HOLD) { + if (ctrl_reg & CDNS_I2C_CR_HOLD) { hold_clear = true; if (id->quirks & CDNS_I2C_BROKEN_HOLD_BIT) irq_save = true; @@ -624,7 +624,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) addr &= CDNS_I2C_ADDR_MASK; if (hold_clear) { - ctrl_reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & ~CDNS_I2C_CR_HOLD; + ctrl_reg &= ~CDNS_I2C_CR_HOLD; /* * In case of Xilinx Zynq SOC, clear the HOLD bit before transfer size * register reaches '0'. This is an IP bug which causes transfer size -- 2.30.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] i2c: cadence: Remove unnecessary register reads 2023-01-07 21:18 ` [PATCH 5/5] i2c: cadence: Remove unnecessary register reads Lars-Peter Clausen @ 2023-01-16 14:58 ` Michal Simek 2023-01-16 17:14 ` Lars-Peter Clausen 2023-03-16 19:29 ` Wolfram Sang 1 sibling, 1 reply; 21+ messages in thread From: Michal Simek @ 2023-01-16 14:58 UTC (permalink / raw) To: Lars-Peter Clausen, Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c On 1/7/23 22:18, Lars-Peter Clausen wrote: > > In the `cdns_i2c_mrecv()` function the CTRL register of the Cadence I2C > controller is written and read back multiple times. The register value does > not change on its own. So it is possible to remember the just written value > instead of reading it back from the hardware. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > drivers/i2c/busses/i2c-cadence.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c > index bec50bfe7aad..93c6d0822468 100644 > --- a/drivers/i2c/busses/i2c-cadence.c > +++ b/drivers/i2c/busses/i2c-cadence.c > @@ -613,7 +613,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) > > /* Determine hold_clear based on number of bytes to receive and hold flag */ > if (!id->bus_hold_flag && id->recv_count <= CDNS_I2C_FIFO_DEPTH) { > - if (cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & CDNS_I2C_CR_HOLD) { > + if (ctrl_reg & CDNS_I2C_CR_HOLD) { > hold_clear = true; > if (id->quirks & CDNS_I2C_BROKEN_HOLD_BIT) > irq_save = true; > @@ -624,7 +624,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) > addr &= CDNS_I2C_ADDR_MASK; > > if (hold_clear) { > - ctrl_reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & ~CDNS_I2C_CR_HOLD; > + ctrl_reg &= ~CDNS_I2C_CR_HOLD; > /* > * In case of Xilinx Zynq SOC, clear the HOLD bit before transfer size > * register reaches '0'. This is an IP bug which causes transfer size > -- > 2.30.2 > Logically this is fine but that additional read on CR register ensures that IP receive previous writes. The code itself is related to bug on Zynq SoC and that two additional readbacks can actually do something. I think this should be properly tested on zynq to ensure that it doesn't break anything. Shubhrajyoti: Can you please make sure that it is tested on Zynq? Thanks, Michal ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] i2c: cadence: Remove unnecessary register reads 2023-01-16 14:58 ` Michal Simek @ 2023-01-16 17:14 ` Lars-Peter Clausen 2023-02-27 10:47 ` Michal Simek 0 siblings, 1 reply; 21+ messages in thread From: Lars-Peter Clausen @ 2023-01-16 17:14 UTC (permalink / raw) To: Michal Simek, Wolfram Sang; +Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c On 1/16/23 06:58, Michal Simek wrote: > > > On 1/7/23 22:18, Lars-Peter Clausen wrote: >> >> In the `cdns_i2c_mrecv()` function the CTRL register of the Cadence I2C >> controller is written and read back multiple times. The register >> value does >> not change on its own. So it is possible to remember the just written >> value >> instead of reading it back from the hardware. >> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> >> --- >> drivers/i2c/busses/i2c-cadence.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-cadence.c >> b/drivers/i2c/busses/i2c-cadence.c >> index bec50bfe7aad..93c6d0822468 100644 >> --- a/drivers/i2c/busses/i2c-cadence.c >> +++ b/drivers/i2c/busses/i2c-cadence.c >> @@ -613,7 +613,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) >> >> /* Determine hold_clear based on number of bytes to receive >> and hold flag */ >> if (!id->bus_hold_flag && id->recv_count <= >> CDNS_I2C_FIFO_DEPTH) { >> - if (cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & >> CDNS_I2C_CR_HOLD) { >> + if (ctrl_reg & CDNS_I2C_CR_HOLD) { >> hold_clear = true; >> if (id->quirks & CDNS_I2C_BROKEN_HOLD_BIT) >> irq_save = true; >> @@ -624,7 +624,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) >> addr &= CDNS_I2C_ADDR_MASK; >> >> if (hold_clear) { >> - ctrl_reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & >> ~CDNS_I2C_CR_HOLD; >> + ctrl_reg &= ~CDNS_I2C_CR_HOLD; >> /* >> * In case of Xilinx Zynq SOC, clear the HOLD bit >> before transfer size >> * register reaches '0'. This is an IP bug which >> causes transfer size >> -- >> 2.30.2 >> > > Logically this is fine but that additional read on CR register ensures > that IP receive previous writes. The code itself is related to bug on > Zynq SoC and that two additional readbacks can actually do something. > > I think this should be properly tested on zynq to ensure that it > doesn't break anything. > > Shubhrajyoti: Can you please make sure that it is tested on Zynq? Maybe it is better to drop the patch then if it is used to enforce ordering in the hardware. But I guess we should add a comment to explain this. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] i2c: cadence: Remove unnecessary register reads 2023-01-16 17:14 ` Lars-Peter Clausen @ 2023-02-27 10:47 ` Michal Simek 2023-03-16 19:28 ` Wolfram Sang 0 siblings, 1 reply; 21+ messages in thread From: Michal Simek @ 2023-02-27 10:47 UTC (permalink / raw) To: Lars-Peter Clausen, Wolfram Sang Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c On 1/16/23 18:14, Lars-Peter Clausen wrote: > On 1/16/23 06:58, Michal Simek wrote: >> >> >> On 1/7/23 22:18, Lars-Peter Clausen wrote: >>> >>> In the `cdns_i2c_mrecv()` function the CTRL register of the Cadence I2C >>> controller is written and read back multiple times. The register value does >>> not change on its own. So it is possible to remember the just written value >>> instead of reading it back from the hardware. >>> >>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> >>> --- >>> drivers/i2c/busses/i2c-cadence.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c >>> index bec50bfe7aad..93c6d0822468 100644 >>> --- a/drivers/i2c/busses/i2c-cadence.c >>> +++ b/drivers/i2c/busses/i2c-cadence.c >>> @@ -613,7 +613,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) >>> >>> /* Determine hold_clear based on number of bytes to receive and hold >>> flag */ >>> if (!id->bus_hold_flag && id->recv_count <= CDNS_I2C_FIFO_DEPTH) { >>> - if (cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & CDNS_I2C_CR_HOLD) { >>> + if (ctrl_reg & CDNS_I2C_CR_HOLD) { >>> hold_clear = true; >>> if (id->quirks & CDNS_I2C_BROKEN_HOLD_BIT) >>> irq_save = true; >>> @@ -624,7 +624,7 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id) >>> addr &= CDNS_I2C_ADDR_MASK; >>> >>> if (hold_clear) { >>> - ctrl_reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & >>> ~CDNS_I2C_CR_HOLD; >>> + ctrl_reg &= ~CDNS_I2C_CR_HOLD; >>> /* >>> * In case of Xilinx Zynq SOC, clear the HOLD bit before >>> transfer size >>> * register reaches '0'. This is an IP bug which causes >>> transfer size >>> -- >>> 2.30.2 >>> >> >> Logically this is fine but that additional read on CR register ensures that IP >> receive previous writes. The code itself is related to bug on Zynq SoC and >> that two additional readbacks can actually do something. >> >> I think this should be properly tested on zynq to ensure that it doesn't break >> anything. >> >> Shubhrajyoti: Can you please make sure that it is tested on Zynq? > Maybe it is better to drop the patch then if it is used to enforce ordering in > the hardware. But I guess we should add a comment to explain this. Mani has tested it and he can't see any issue that's why I am fine both ways. Thanks, Michal ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] i2c: cadence: Remove unnecessary register reads 2023-02-27 10:47 ` Michal Simek @ 2023-03-16 19:28 ` Wolfram Sang 0 siblings, 0 replies; 21+ messages in thread From: Wolfram Sang @ 2023-03-16 19:28 UTC (permalink / raw) To: Michal Simek Cc: Lars-Peter Clausen, Michal Simek, Shubhrajyoti Datta, linux-i2c [-- Attachment #1: Type: text/plain, Size: 106 bytes --] > Mani has tested it and he can't see any issue that's why I am fine both ways. I read this as an ack. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] i2c: cadence: Remove unnecessary register reads 2023-01-07 21:18 ` [PATCH 5/5] i2c: cadence: Remove unnecessary register reads Lars-Peter Clausen 2023-01-16 14:58 ` Michal Simek @ 2023-03-16 19:29 ` Wolfram Sang 1 sibling, 0 replies; 21+ messages in thread From: Wolfram Sang @ 2023-03-16 19:29 UTC (permalink / raw) To: Lars-Peter Clausen; +Cc: Michal Simek, Shubhrajyoti Datta, linux-i2c [-- Attachment #1: Type: text/plain, Size: 444 bytes --] On Sat, Jan 07, 2023 at 01:18:14PM -0800, Lars-Peter Clausen wrote: > In the `cdns_i2c_mrecv()` function the CTRL register of the Cadence I2C > controller is written and read back multiple times. The register value does > not change on its own. So it is possible to remember the just written value > instead of reading it back from the hardware. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Applied to for-next, thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <BY5PR12MB4902E8FC89477C6A1038F15281A09@BY5PR12MB4902.namprd12.prod.outlook.com>]
* RE: [PATCH 0/5] i2c: cadence: Small cleanups [not found] ` <BY5PR12MB4902E8FC89477C6A1038F15281A09@BY5PR12MB4902.namprd12.prod.outlook.com> @ 2023-02-16 5:18 ` Guntupalli, Manikanta 0 siblings, 0 replies; 21+ messages in thread From: Guntupalli, Manikanta @ 2023-02-16 5:18 UTC (permalink / raw) To: linux-i2c; +Cc: michal.simek, Datta, Shubhrajyoti, lars, wsa Hi , > -----Original Message----- > From: Lars-Peter Clausen <lars@metafoo.de> > Sent: Sunday, January 8, 2023 2:48 AM > To: Wolfram Sang <wsa@kernel.org> > Cc: Michal Simek <michal.simek@xilinx.com>; Datta, Shubhrajyoti > <shubhrajyoti.datta@amd.com>; linux-i2c@vger.kernel.org; Lars-Peter > Clausen <lars@metafoo.de> > Subject: [PATCH 0/5] i2c: cadence: Small cleanups > > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > This series has a set of small cleanups for the Cadence I2C driver. The > changes are mostly cosmetical and there is no change in behavior of the > driver. > Tested-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com> > Lars-Peter Clausen (5): > i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define > i2c: cadence: Remove `irq` field from driver state struct > i2c: cadence: Remove redundant expression in if clause > i2c: cadence: Remove always false ternary operator > i2c: cadence: Remove unnecessary register reads > > drivers/i2c/busses/i2c-cadence.c | 30 +++++++++++------------------- > 1 file changed, 11 insertions(+), 19 deletions(-) > > -- > 2.30.2 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-03-16 19:29 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-07 21:18 [PATCH 0/5] i2c: cadence: Small cleanups Lars-Peter Clausen
2023-01-07 21:18 ` [PATCH 1/5] i2c: cadence: Remove unused CDNS_I2C_DATA_INTR_DEPTH define Lars-Peter Clausen
2023-01-16 14:37 ` Michal Simek
2023-01-20 8:56 ` Wolfram Sang
2023-01-07 21:18 ` [PATCH 2/5] i2c: cadence: Remove `irq` field from driver state struct Lars-Peter Clausen
2023-01-16 14:39 ` Michal Simek
2023-01-20 8:56 ` Wolfram Sang
2023-01-07 21:18 ` [PATCH 3/5] i2c: cadence: Remove redundant expression in if clause Lars-Peter Clausen
2023-01-16 14:44 ` Michal Simek
2023-01-20 8:56 ` Wolfram Sang
2023-01-07 21:18 ` [PATCH 4/5] i2c: cadence: Remove always false ternary operator Lars-Peter Clausen
2023-01-16 15:00 ` Michal Simek
2023-02-27 10:46 ` Michal Simek
2023-03-16 19:29 ` Wolfram Sang
2023-01-07 21:18 ` [PATCH 5/5] i2c: cadence: Remove unnecessary register reads Lars-Peter Clausen
2023-01-16 14:58 ` Michal Simek
2023-01-16 17:14 ` Lars-Peter Clausen
2023-02-27 10:47 ` Michal Simek
2023-03-16 19:28 ` Wolfram Sang
2023-03-16 19:29 ` Wolfram Sang
[not found] ` <BY5PR12MB4902E8FC89477C6A1038F15281A09@BY5PR12MB4902.namprd12.prod.outlook.com>
2023-02-16 5:18 ` [PATCH 0/5] i2c: cadence: Small cleanups Guntupalli, Manikanta
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.