* [PATCH v1] i2c: designware: Change from u32 to unsigned int for regmap_read() calls
@ 2023-01-24 11:47 Shyam Sundar S K
2023-01-24 12:54 ` Jarkko Nikula
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Shyam Sundar S K @ 2023-01-24 11:47 UTC (permalink / raw)
To: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, Jan Dabros
Cc: linux-i2c, Shyam Sundar S K
regmap_read() API signature expects the caller to send "unsigned int"
type to return back the read value, but there are some occurrences of 'u32'
across i2c-designware-* files.
Change them to match the regmap_read() signature.
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/i2c/busses/i2c-designware-common.c | 11 ++++++-----
drivers/i2c/busses/i2c-designware-core.h | 2 +-
drivers/i2c/busses/i2c-designware-master.c | 13 +++++++------
drivers/i2c/busses/i2c-designware-slave.c | 4 ++--
4 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index a3240ece55b2..ae808e91b17f 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -388,7 +388,7 @@ u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
{
- u32 reg;
+ unsigned int reg;
int ret;
ret = i2c_dw_acquire_lock(dev);
@@ -439,7 +439,7 @@ int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
void __i2c_dw_disable(struct dw_i2c_dev *dev)
{
int timeout = 100;
- u32 status;
+ unsigned int status;
do {
__i2c_dw_disable_nowait(dev);
@@ -524,7 +524,7 @@ void i2c_dw_release_lock(struct dw_i2c_dev *dev)
*/
int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
{
- u32 status;
+ unsigned int status;
int ret;
ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
@@ -568,7 +568,8 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
{
- u32 param, tx_fifo_depth, rx_fifo_depth;
+ u32 tx_fifo_depth, rx_fifo_depth;
+ unsigned int param;
int ret;
/*
@@ -608,7 +609,7 @@ u32 i2c_dw_func(struct i2c_adapter *adap)
void i2c_dw_disable(struct dw_i2c_dev *dev)
{
- u32 dummy;
+ unsigned int dummy;
int ret;
ret = i2c_dw_acquire_lock(dev);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index a7ec8d5d0e72..903d5928e230 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -265,7 +265,7 @@ struct dw_i2c_dev {
u8 *rx_buf;
int msg_err;
unsigned int status;
- u32 abort_source;
+ unsigned int abort_source;
int irq;
u32 flags;
struct i2c_adapter adapter;
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 3be7581ee3fb..55ea91a63382 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -39,7 +39,7 @@ static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
{
- u32 comp_param1;
+ unsigned int comp_param1;
u32 sda_falling_time, scl_falling_time;
struct i2c_timings *t = &dev->timings;
const char *fp_str = "";
@@ -211,7 +211,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
{
struct i2c_msg *msgs = dev->msgs;
u32 ic_con = 0, ic_tar = 0;
- u32 dummy;
+ unsigned int dummy;
/* Disable the adapter */
__i2c_dw_disable(dev);
@@ -287,7 +287,7 @@ static int amd_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs,
int msg_wrt_idx, msg_itr_lmt, buf_len, data_idx;
int cmd = 0, status;
u8 *tx_buf;
- u32 val;
+ unsigned int val;
/*
* In order to enable the interrupt for UCSI i.e. AMD NAVI GPU card,
@@ -505,7 +505,8 @@ i2c_dw_read(struct dw_i2c_dev *dev)
unsigned int rx_valid;
for (; dev->msg_read_idx < dev->msgs_num; dev->msg_read_idx++) {
- u32 len, tmp;
+ unsigned int tmp;
+ u32 len;
u8 *buf;
if (!(msgs[dev->msg_read_idx].flags & I2C_M_RD))
@@ -653,7 +654,7 @@ static const struct i2c_adapter_quirks i2c_dw_quirks = {
static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
{
- u32 stat, dummy;
+ unsigned int stat, dummy;
/*
* The IC_INTR_STAT register just indicates "enabled" interrupts.
@@ -714,7 +715,7 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
{
struct dw_i2c_dev *dev = dev_id;
- u32 stat, enabled;
+ unsigned int stat, enabled;
regmap_read(dev->map, DW_IC_ENABLE, &enabled);
regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index c6d2e4c2ac23..cec25054bb24 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -98,7 +98,7 @@ static int i2c_dw_unreg_slave(struct i2c_client *slave)
static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
{
- u32 stat, dummy;
+ unsigned int stat, dummy;
/*
* The IC_INTR_STAT register just indicates "enabled" interrupts.
@@ -150,7 +150,7 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
{
struct dw_i2c_dev *dev = dev_id;
- u32 raw_stat, stat, enabled, tmp;
+ unsigned int raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
regmap_read(dev->map, DW_IC_ENABLE, &enabled);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v1] i2c: designware: Change from u32 to unsigned int for regmap_read() calls
2023-01-24 11:47 [PATCH v1] i2c: designware: Change from u32 to unsigned int for regmap_read() calls Shyam Sundar S K
@ 2023-01-24 12:54 ` Jarkko Nikula
2023-01-24 13:02 ` Andy Shevchenko
2023-01-24 12:58 ` Andy Shevchenko
2023-01-28 18:47 ` Wolfram Sang
2 siblings, 1 reply; 6+ messages in thread
From: Jarkko Nikula @ 2023-01-24 12:54 UTC (permalink / raw)
To: Shyam Sundar S K, Andy Shevchenko, Mika Westerberg, Jan Dabros; +Cc: linux-i2c
On 1/24/23 13:47, Shyam Sundar S K wrote:
> regmap_read() API signature expects the caller to send "unsigned int"
> type to return back the read value, but there are some occurrences of 'u32'
> across i2c-designware-* files.
>
> Change them to match the regmap_read() signature.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/i2c/busses/i2c-designware-common.c | 11 ++++++-----
> drivers/i2c/busses/i2c-designware-core.h | 2 +-
> drivers/i2c/busses/i2c-designware-master.c | 13 +++++++------
> drivers/i2c/busses/i2c-designware-slave.c | 4 ++--
> 4 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index a3240ece55b2..ae808e91b17f 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -388,7 +388,7 @@ u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
>
> int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
> {
> - u32 reg;
> + unsigned int reg;
> int ret;
>
Hmm.. I'm not sure about these. We know registers are 32-bit and change
to unsigned int is a step being more ambiguous. I'm wearing my old
embedded developer hat who likes to see explicit types when dealing with HW.
Andy: what was your rationale to propose changing u32 to unsigned int in
another i2c-designware patch? Has gcc started complaining if
regmap_read() is used with u32 type?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] i2c: designware: Change from u32 to unsigned int for regmap_read() calls
2023-01-24 12:54 ` Jarkko Nikula
@ 2023-01-24 13:02 ` Andy Shevchenko
2023-01-26 13:44 ` Jarkko Nikula
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2023-01-24 13:02 UTC (permalink / raw)
To: Jarkko Nikula; +Cc: Shyam Sundar S K, Mika Westerberg, Jan Dabros, linux-i2c
On Tue, Jan 24, 2023 at 02:54:52PM +0200, Jarkko Nikula wrote:
> On 1/24/23 13:47, Shyam Sundar S K wrote:
> > regmap_read() API signature expects the caller to send "unsigned int"
> > type to return back the read value, but there are some occurrences of 'u32'
> > across i2c-designware-* files.
> >
> > Change them to match the regmap_read() signature.
...
> Hmm.. I'm not sure about these. We know registers are 32-bit and change to
> unsigned int is a step being more ambiguous. I'm wearing my old embedded
> developer hat who likes to see explicit types when dealing with HW.
>
> Andy: what was your rationale to propose changing u32 to unsigned int in
> another i2c-designware patch? Has gcc started complaining if regmap_read()
> is used with u32 type?
To be the same type as regmap API is expecting.
What you are talking about makes sense for the direct IO accessor calls.
Actually something like this had to be done when driver was converted to
regmap APIs.
Here we might have 16-bit registers, they are fine when variable is u32,
but goes over boundaries if we declare it as u16. So unsigned int makes
more sense to me.
OTOH I agree that this change is subjective since there is no functional
or other changes as long as we have unsigned int == u32.
So, up to you, guys. But I'm fine with the change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] i2c: designware: Change from u32 to unsigned int for regmap_read() calls
2023-01-24 13:02 ` Andy Shevchenko
@ 2023-01-26 13:44 ` Jarkko Nikula
0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Nikula @ 2023-01-26 13:44 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Shyam Sundar S K, Mika Westerberg, Jan Dabros, linux-i2c
On 1/24/23 15:02, Andy Shevchenko wrote:
> On Tue, Jan 24, 2023 at 02:54:52PM +0200, Jarkko Nikula wrote:
>> On 1/24/23 13:47, Shyam Sundar S K wrote:
>>> regmap_read() API signature expects the caller to send "unsigned int"
>>> type to return back the read value, but there are some occurrences of 'u32'
>>> across i2c-designware-* files.
>>>
>>> Change them to match the regmap_read() signature.
>
> ...
>
>> Hmm.. I'm not sure about these. We know registers are 32-bit and change to
>> unsigned int is a step being more ambiguous. I'm wearing my old embedded
>> developer hat who likes to see explicit types when dealing with HW.
>>
>> Andy: what was your rationale to propose changing u32 to unsigned int in
>> another i2c-designware patch? Has gcc started complaining if regmap_read()
>> is used with u32 type?
>
> To be the same type as regmap API is expecting.
>
> What you are talking about makes sense for the direct IO accessor calls.
> Actually something like this had to be done when driver was converted to
> regmap APIs.
>
> Here we might have 16-bit registers, they are fine when variable is u32,
> but goes over boundaries if we declare it as u16. So unsigned int makes
> more sense to me.
>
> OTOH I agree that this change is subjective since there is no functional
> or other changes as long as we have unsigned int == u32.
>
> So, up to you, guys. But I'm fine with the change.
>
Fair point.
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] i2c: designware: Change from u32 to unsigned int for regmap_read() calls
2023-01-24 11:47 [PATCH v1] i2c: designware: Change from u32 to unsigned int for regmap_read() calls Shyam Sundar S K
2023-01-24 12:54 ` Jarkko Nikula
@ 2023-01-24 12:58 ` Andy Shevchenko
2023-01-28 18:47 ` Wolfram Sang
2 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2023-01-24 12:58 UTC (permalink / raw)
To: Shyam Sundar S K; +Cc: Jarkko Nikula, Mika Westerberg, Jan Dabros, linux-i2c
On Tue, Jan 24, 2023 at 05:17:32PM +0530, Shyam Sundar S K wrote:
> regmap_read() API signature expects the caller to send "unsigned int"
> type to return back the read value, but there are some occurrences of 'u32'
> across i2c-designware-* files.
>
> Change them to match the regmap_read() signature.
So, the keyword is "to bring a consistency".
FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/i2c/busses/i2c-designware-common.c | 11 ++++++-----
> drivers/i2c/busses/i2c-designware-core.h | 2 +-
> drivers/i2c/busses/i2c-designware-master.c | 13 +++++++------
> drivers/i2c/busses/i2c-designware-slave.c | 4 ++--
> 4 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index a3240ece55b2..ae808e91b17f 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -388,7 +388,7 @@ u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
>
> int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
> {
> - u32 reg;
> + unsigned int reg;
> int ret;
>
> ret = i2c_dw_acquire_lock(dev);
> @@ -439,7 +439,7 @@ int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
> void __i2c_dw_disable(struct dw_i2c_dev *dev)
> {
> int timeout = 100;
> - u32 status;
> + unsigned int status;
>
> do {
> __i2c_dw_disable_nowait(dev);
> @@ -524,7 +524,7 @@ void i2c_dw_release_lock(struct dw_i2c_dev *dev)
> */
> int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
> {
> - u32 status;
> + unsigned int status;
> int ret;
>
> ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
> @@ -568,7 +568,8 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
>
> int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
> {
> - u32 param, tx_fifo_depth, rx_fifo_depth;
> + u32 tx_fifo_depth, rx_fifo_depth;
> + unsigned int param;
> int ret;
>
> /*
> @@ -608,7 +609,7 @@ u32 i2c_dw_func(struct i2c_adapter *adap)
>
> void i2c_dw_disable(struct dw_i2c_dev *dev)
> {
> - u32 dummy;
> + unsigned int dummy;
> int ret;
>
> ret = i2c_dw_acquire_lock(dev);
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index a7ec8d5d0e72..903d5928e230 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -265,7 +265,7 @@ struct dw_i2c_dev {
> u8 *rx_buf;
> int msg_err;
> unsigned int status;
> - u32 abort_source;
> + unsigned int abort_source;
> int irq;
> u32 flags;
> struct i2c_adapter adapter;
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index 3be7581ee3fb..55ea91a63382 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -39,7 +39,7 @@ static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
>
> static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
> {
> - u32 comp_param1;
> + unsigned int comp_param1;
> u32 sda_falling_time, scl_falling_time;
> struct i2c_timings *t = &dev->timings;
> const char *fp_str = "";
> @@ -211,7 +211,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
> {
> struct i2c_msg *msgs = dev->msgs;
> u32 ic_con = 0, ic_tar = 0;
> - u32 dummy;
> + unsigned int dummy;
>
> /* Disable the adapter */
> __i2c_dw_disable(dev);
> @@ -287,7 +287,7 @@ static int amd_i2c_dw_xfer_quirk(struct i2c_adapter *adap, struct i2c_msg *msgs,
> int msg_wrt_idx, msg_itr_lmt, buf_len, data_idx;
> int cmd = 0, status;
> u8 *tx_buf;
> - u32 val;
> + unsigned int val;
>
> /*
> * In order to enable the interrupt for UCSI i.e. AMD NAVI GPU card,
> @@ -505,7 +505,8 @@ i2c_dw_read(struct dw_i2c_dev *dev)
> unsigned int rx_valid;
>
> for (; dev->msg_read_idx < dev->msgs_num; dev->msg_read_idx++) {
> - u32 len, tmp;
> + unsigned int tmp;
> + u32 len;
> u8 *buf;
>
> if (!(msgs[dev->msg_read_idx].flags & I2C_M_RD))
> @@ -653,7 +654,7 @@ static const struct i2c_adapter_quirks i2c_dw_quirks = {
>
> static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
> {
> - u32 stat, dummy;
> + unsigned int stat, dummy;
>
> /*
> * The IC_INTR_STAT register just indicates "enabled" interrupts.
> @@ -714,7 +715,7 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
> static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
> {
> struct dw_i2c_dev *dev = dev_id;
> - u32 stat, enabled;
> + unsigned int stat, enabled;
>
> regmap_read(dev->map, DW_IC_ENABLE, &enabled);
> regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
> diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
> index c6d2e4c2ac23..cec25054bb24 100644
> --- a/drivers/i2c/busses/i2c-designware-slave.c
> +++ b/drivers/i2c/busses/i2c-designware-slave.c
> @@ -98,7 +98,7 @@ static int i2c_dw_unreg_slave(struct i2c_client *slave)
>
> static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
> {
> - u32 stat, dummy;
> + unsigned int stat, dummy;
>
> /*
> * The IC_INTR_STAT register just indicates "enabled" interrupts.
> @@ -150,7 +150,7 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
> static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
> {
> struct dw_i2c_dev *dev = dev_id;
> - u32 raw_stat, stat, enabled, tmp;
> + unsigned int raw_stat, stat, enabled, tmp;
> u8 val = 0, slave_activity;
>
> regmap_read(dev->map, DW_IC_ENABLE, &enabled);
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] i2c: designware: Change from u32 to unsigned int for regmap_read() calls
2023-01-24 11:47 [PATCH v1] i2c: designware: Change from u32 to unsigned int for regmap_read() calls Shyam Sundar S K
2023-01-24 12:54 ` Jarkko Nikula
2023-01-24 12:58 ` Andy Shevchenko
@ 2023-01-28 18:47 ` Wolfram Sang
2 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2023-01-28 18:47 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, Jan Dabros,
linux-i2c
[-- Attachment #1: Type: text/plain, Size: 410 bytes --]
On Tue, Jan 24, 2023 at 05:17:32PM +0530, Shyam Sundar S K wrote:
> regmap_read() API signature expects the caller to send "unsigned int"
> type to return back the read value, but there are some occurrences of 'u32'
> across i2c-designware-* files.
>
> Change them to match the regmap_read() signature.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-28 18:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-24 11:47 [PATCH v1] i2c: designware: Change from u32 to unsigned int for regmap_read() calls Shyam Sundar S K
2023-01-24 12:54 ` Jarkko Nikula
2023-01-24 13:02 ` Andy Shevchenko
2023-01-26 13:44 ` Jarkko Nikula
2023-01-24 12:58 ` Andy Shevchenko
2023-01-28 18:47 ` Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox