| Message ID | 20240819-i3c_fix-v3-2-7d69f7b0a05e@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i3c: master: some fix and improvemnt for hotjoin | expand |
Hi Frank, Frank.Li@nxp.com wrote on Mon, 19 Aug 2024 12:01:56 -0400: > Replace the hardcoded value 2, which indicates 2 bits for I3C address > status, with the predefined macro I3C_ADDR_SLOT_BITS. I'm fine with the idea but I don't understand the macro name. You're talking about status bits and yet the macro is named addr_slot? > Improve maintainability and extensibility of the code. Thanks, Miquèl
On Fri, Aug 23, 2024 at 05:55:02PM +0200, Miquel Raynal wrote: > Hi Frank, > > Frank.Li@nxp.com wrote on Mon, 19 Aug 2024 12:01:56 -0400: > > > Replace the hardcoded value 2, which indicates 2 bits for I3C address > > status, with the predefined macro I3C_ADDR_SLOT_BITS. > > I'm fine with the idea but I don't understand the macro name. You're > talking about status bits and yet the macro is named addr_slot? > How about I3C_ADDR_SLOT_STATUS_BITS ? > > Improve maintainability and extensibility of the code. > > Thanks, > Miquèl
Hi Frank, Frank.li@nxp.com wrote on Fri, 23 Aug 2024 13:57:31 -0400: > On Fri, Aug 23, 2024 at 05:55:02PM +0200, Miquel Raynal wrote: > > Hi Frank, > > > > Frank.Li@nxp.com wrote on Mon, 19 Aug 2024 12:01:56 -0400: > > > > > Replace the hardcoded value 2, which indicates 2 bits for I3C address > > > status, with the predefined macro I3C_ADDR_SLOT_BITS. > > > > I'm fine with the idea but I don't understand the macro name. You're > > talking about status bits and yet the macro is named addr_slot? > > > > How about I3C_ADDR_SLOT_STATUS_BITS ? > > > > Improve maintainability and extensibility of the code. I'm a bit flaky on this concept, but let's try that. Perhaps with all the changes requested (esp. rephrasing) it might become clearer in next version. Thanks, Miquèl
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 852b32178b722..85c737554c940 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -348,7 +348,7 @@ static enum i3c_addr_slot_status i3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr) { unsigned long status; - int bitpos = addr * 2; + int bitpos = addr * I3C_ADDR_SLOT_BITS; if (addr > I2C_MAX_ADDR) return I3C_ADDR_SLOT_RSVD; @@ -362,7 +362,7 @@ i3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr) static void i3c_bus_set_addr_slot_status(struct i3c_bus *bus, u16 addr, enum i3c_addr_slot_status status) { - int bitpos = addr * 2; + int bitpos = addr * I3C_ADDR_SLOT_BITS; unsigned long *ptr; if (addr > I2C_MAX_ADDR) diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h index 074f632868d98..4601b6957f799 100644 --- a/include/linux/i3c/master.h +++ b/include/linux/i3c/master.h @@ -299,6 +299,8 @@ enum i3c_addr_slot_status { I3C_ADDR_SLOT_STATUS_MASK = 3, }; +#define I3C_ADDR_SLOT_BITS 2 + /** * struct i3c_bus - I3C bus object * @cur_master: I3C master currently driving the bus. Since I3C is multi-master @@ -340,7 +342,7 @@ enum i3c_addr_slot_status { struct i3c_bus { struct i3c_dev_desc *cur_master; int id; - unsigned long addrslots[((I2C_MAX_ADDR + 1) * 2) / BITS_PER_LONG]; + unsigned long addrslots[((I2C_MAX_ADDR + 1) * I3C_ADDR_SLOT_BITS) / BITS_PER_LONG]; enum i3c_bus_mode mode; struct { unsigned long i3c;
Replace the hardcoded value 2, which indicates 2 bits for I3C address status, with the predefined macro I3C_ADDR_SLOT_BITS. Improve maintainability and extensibility of the code. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/i3c/master.c | 4 ++-- include/linux/i3c/master.h | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-)