mbox series

[v2,0/7] i2c: npcm: Bug fixes read/write operation, checkpatch

Message ID 20240830034640.7049-1-kfting@nuvoton.com
Headers show
Series i2c: npcm: Bug fixes read/write operation, checkpatch | expand

Message

Tyrone Ting Aug. 30, 2024, 3:46 a.m. UTC
This patchset includes the following fixes:

- Restore the npcm_i2caddr array length to fix the smatch warning.
- Enable the target functionality in the interrupt handling routine 
  when the i2c transfer is about to finish.
- Correct the read/write operation procedure.
- Introduce a software flag to handle the bus error (BER) condition
  which is not caused by the i2c transfer.
- Modify timeout calculation.
- Assign the client address earlier logically.
- Use an i2c frequency table for the frequency parameters assignment.
- Coding style fix.

The NPCM I2C driver is tested on NPCM750 and NPCM845 evaluation boards.

Addressed comments from:
- kernel test robot : https://lore.kernel.org/oe-kbuild-all/
  202408080319.de2B6PgU-lkp@intel.com/
- Dan Carpenter : https://lore.kernel.org/all/202408130818
  .FgDP5uNm-lkp@intel.com/
- Andrew Jeffery : https://lore.kernel.org/lkml/
  20240807100244.16872-7-kfting@nuvoton.com/T/
  #m3ed3351bf59675bfe0de89c75aae1fb26cad5567

Changes since version 1:
- Restore the npcm_i2caddr array length to fix the smatch warning.
- Remove unused variables.
- Handle the condition where scl_table_cnt reaches to the maximum value.
- Fix the checkpatch warning.

Charles Boyer (1):
  i2c: npcm: Enable slave in eob interrupt

Tyrone Ting (6):
  i2c: npcm: restore slave addresses array length
  i2c: npcm: correct the read/write operation procedure
  i2c: npcm: use a software flag to indicate a BER condition
  i2c: npcm: Modify timeout evaluation mechanism
  i2c: npcm: Modify the client address assignment
  i2c: npcm: use i2c frequency table

 drivers/i2c/busses/i2c-npcm7xx.c | 276 +++++++++++++++++++------------
 1 file changed, 172 insertions(+), 104 deletions(-)


base-commit: 5be63fc19fcaa4c236b307420483578a56986a37

Comments

Tali Perry Sept. 8, 2024, 10:58 a.m. UTC | #1
Reviewed-by: Tali Perry <tali.perry1@gmail.com>

On Fri, Aug 30, 2024 at 6:48 AM Tyrone Ting <warp5tw@gmail.com> wrote:
>
> This patchset includes the following fixes:
>
> - Restore the npcm_i2caddr array length to fix the smatch warning.
> - Enable the target functionality in the interrupt handling routine
>   when the i2c transfer is about to finish.
> - Correct the read/write operation procedure.
> - Introduce a software flag to handle the bus error (BER) condition
>   which is not caused by the i2c transfer.
> - Modify timeout calculation.
> - Assign the client address earlier logically.
> - Use an i2c frequency table for the frequency parameters assignment.
> - Coding style fix.
>
> The NPCM I2C driver is tested on NPCM750 and NPCM845 evaluation boards.
>
> Addressed comments from:
> - kernel test robot : https://lore.kernel.org/oe-kbuild-all/
>   202408080319.de2B6PgU-lkp@intel.com/
> - Dan Carpenter : https://lore.kernel.org/all/202408130818
>   .FgDP5uNm-lkp@intel.com/
> - Andrew Jeffery : https://lore.kernel.org/lkml/
>   20240807100244.16872-7-kfting@nuvoton.com/T/
>   #m3ed3351bf59675bfe0de89c75aae1fb26cad5567
>
> Changes since version 1:
> - Restore the npcm_i2caddr array length to fix the smatch warning.
> - Remove unused variables.
> - Handle the condition where scl_table_cnt reaches to the maximum value.
> - Fix the checkpatch warning.
>
> Charles Boyer (1):
>   i2c: npcm: Enable slave in eob interrupt
>
> Tyrone Ting (6):
>   i2c: npcm: restore slave addresses array length
>   i2c: npcm: correct the read/write operation procedure
>   i2c: npcm: use a software flag to indicate a BER condition
>   i2c: npcm: Modify timeout evaluation mechanism
>   i2c: npcm: Modify the client address assignment
>   i2c: npcm: use i2c frequency table
>
>  drivers/i2c/busses/i2c-npcm7xx.c | 276 +++++++++++++++++++------------
>  1 file changed, 172 insertions(+), 104 deletions(-)
>
>
> base-commit: 5be63fc19fcaa4c236b307420483578a56986a37
> --
> 2.34.1
>
Andi Shyti Sept. 9, 2024, 1 p.m. UTC | #2
Hi,

On Fri, Aug 30, 2024 at 11:46:33AM GMT, Tyrone Ting wrote:
> This patchset includes the following fixes:
> 
> - Restore the npcm_i2caddr array length to fix the smatch warning.
> - Enable the target functionality in the interrupt handling routine 
>   when the i2c transfer is about to finish.
> - Correct the read/write operation procedure.
> - Introduce a software flag to handle the bus error (BER) condition
>   which is not caused by the i2c transfer.
> - Modify timeout calculation.
> - Assign the client address earlier logically.
> - Use an i2c frequency table for the frequency parameters assignment.
> - Coding style fix.
> 
> The NPCM I2C driver is tested on NPCM750 and NPCM845 evaluation boards.

first of all, Thanks Tali for your reviews.

Second, Tyronne, can we please incorporate Tali's comments into
commit messages and improve the code comments so that we don't
leave room for more questions?

Consider that not everyone knows the device and we need good
reasons for accepting the changes.

Thanks,
Andi


> Addressed comments from:
> - kernel test robot : https://lore.kernel.org/oe-kbuild-all/
>   202408080319.de2B6PgU-lkp@intel.com/
> - Dan Carpenter : https://lore.kernel.org/all/202408130818
>   .FgDP5uNm-lkp@intel.com/
> - Andrew Jeffery : https://lore.kernel.org/lkml/
>   20240807100244.16872-7-kfting@nuvoton.com/T/
>   #m3ed3351bf59675bfe0de89c75aae1fb26cad5567
> 
> Changes since version 1:
> - Restore the npcm_i2caddr array length to fix the smatch warning.
> - Remove unused variables.
> - Handle the condition where scl_table_cnt reaches to the maximum value.
> - Fix the checkpatch warning.
> 
> Charles Boyer (1):
>   i2c: npcm: Enable slave in eob interrupt
> 
> Tyrone Ting (6):
>   i2c: npcm: restore slave addresses array length
>   i2c: npcm: correct the read/write operation procedure
>   i2c: npcm: use a software flag to indicate a BER condition
>   i2c: npcm: Modify timeout evaluation mechanism
>   i2c: npcm: Modify the client address assignment
>   i2c: npcm: use i2c frequency table
> 
>  drivers/i2c/busses/i2c-npcm7xx.c | 276 +++++++++++++++++++------------
>  1 file changed, 172 insertions(+), 104 deletions(-)
> 
> 
> base-commit: 5be63fc19fcaa4c236b307420483578a56986a37
> -- 
> 2.34.1
>
Tyrone Ting Sept. 10, 2024, 1:12 a.m. UTC | #3
Hi Andi:

Thank you for your feedback.

Andi Shyti <andi.shyti@kernel.org> 於 2024年9月9日 週一 下午9:00寫道:
>
> Hi,
>
> On Fri, Aug 30, 2024 at 11:46:33AM GMT, Tyrone Ting wrote:
> > This patchset includes the following fixes:
> >
> > - Restore the npcm_i2caddr array length to fix the smatch warning.
> > - Enable the target functionality in the interrupt handling routine
> >   when the i2c transfer is about to finish.
> > - Correct the read/write operation procedure.
> > - Introduce a software flag to handle the bus error (BER) condition
> >   which is not caused by the i2c transfer.
> > - Modify timeout calculation.
> > - Assign the client address earlier logically.
> > - Use an i2c frequency table for the frequency parameters assignment.
> > - Coding style fix.
> >
> > The NPCM I2C driver is tested on NPCM750 and NPCM845 evaluation boards.
>
> first of all, Thanks Tali for your reviews.
>
> Second, Tyronne, can we please incorporate Tali's comments into
> commit messages and improve the code comments so that we don't
> leave room for more questions?
>
Understood. They'll be incorporated in next patch set.

> Consider that not everyone knows the device and we need good
> reasons for accepting the changes.
>
> Thanks,
> Andi
>
>
> > Addressed comments from:
> > - kernel test robot : https://lore.kernel.org/oe-kbuild-all/
> >   202408080319.de2B6PgU-lkp@intel.com/
> > - Dan Carpenter : https://lore.kernel.org/all/202408130818
> >   .FgDP5uNm-lkp@intel.com/
> > - Andrew Jeffery : https://lore.kernel.org/lkml/
> >   20240807100244.16872-7-kfting@nuvoton.com/T/
> >   #m3ed3351bf59675bfe0de89c75aae1fb26cad5567
> >
> > Changes since version 1:
> > - Restore the npcm_i2caddr array length to fix the smatch warning.
> > - Remove unused variables.
> > - Handle the condition where scl_table_cnt reaches to the maximum value.
> > - Fix the checkpatch warning.
> >
> > Charles Boyer (1):
> >   i2c: npcm: Enable slave in eob interrupt
> >
> > Tyrone Ting (6):
> >   i2c: npcm: restore slave addresses array length
> >   i2c: npcm: correct the read/write operation procedure
> >   i2c: npcm: use a software flag to indicate a BER condition
> >   i2c: npcm: Modify timeout evaluation mechanism
> >   i2c: npcm: Modify the client address assignment
> >   i2c: npcm: use i2c frequency table
> >
> >  drivers/i2c/busses/i2c-npcm7xx.c | 276 +++++++++++++++++++------------
> >  1 file changed, 172 insertions(+), 104 deletions(-)
> >
> >
> > base-commit: 5be63fc19fcaa4c236b307420483578a56986a37
> > --
> > 2.34.1
> >

Thank you.

Regards,
Tyrone