Skip to content

Bl808/i2c proposed fixes#1

Merged
Hunter1753 merged 4 commits intoHunter1753:bl808/i2cfrom
kadamski:bl808/i2c
Mar 29, 2023
Merged

Bl808/i2c proposed fixes#1
Hunter1753 merged 4 commits intoHunter1753:bl808/i2cfrom
kadamski:bl808/i2c

Conversation

@kadamski
Copy link

Just a few of the proposed changed that I noticed while reviewing this code. Please keep in mind that I do not have the hardware to test those changes yet but I still wanted to communicate those ideas to you.

@Hunter1753
Copy link
Owner

hey, i pulled your branch and ran the build and i got some errors:

drivers/i2c/busses/i2c-bl808.c: In function 'bl808_i2c_disable':
drivers/i2c/busses/i2c-bl808.c:428:9: error: implicit declaration of function 'bl808_i2c_clear_fifo_err' [-Werror=implicit-function-declaration]
  428 |         bl808_i2c_clear_fifo_err(i2c_dev);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-bl808.c:429:9: error: implicit declaration of function 'bl808_i2c_clear_interrupts' [-Werror=implicit-function-declaration]
  429 |         bl808_i2c_clear_interrupts(i2c_dev);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-bl808.c: At top level:
drivers/i2c/busses/i2c-bl808.c:492:13: warning: conflicting types for 'bl808_i2c_clear_interrupts'
  492 | static void bl808_i2c_clear_interrupts(struct bl808_i2c_dev *i2c_dev) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-bl808.c:492:13: error: static declaration of 'bl808_i2c_clear_interrupts' follows non-static declaration
drivers/i2c/busses/i2c-bl808.c:429:9: note: previous implicit declaration of 'bl808_i2c_clear_interrupts' was here
  429 |         bl808_i2c_clear_interrupts(i2c_dev);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-bl808.c:500:13: warning: conflicting types for 'bl808_i2c_clear_fifo_err'
  500 | static void bl808_i2c_clear_fifo_err(struct bl808_i2c_dev *i2c_dev) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-bl808.c:500:13: error: static declaration of 'bl808_i2c_clear_fifo_err' follows non-static declaration
drivers/i2c/busses/i2c-bl808.c:428:9: note: previous implicit declaration of 'bl808_i2c_clear_fifo_err' was here
  428 |         bl808_i2c_clear_fifo_err(i2c_dev);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~

Both if statement branches are basically the same and the code can be
deduplicated using the min_t() macro.
The write to BL808_I2C_FIFO_WDATA would happen even if msg_buf_remaining
was 0 and the value to be written was not set, which doesn't make much
sense. This patch fixes that.
Maximal transfer len should be specified using quirks as this is the
standard method. The i2c-core will make sure our master_xfer callback is
not called with messages that are too long.

It is important as some of the i2c clinet code (and regmap code) checks
this field to understand the adapter capabilities.
The TX_FIFO_CLR and RX_FIFO_CLR fileds of the FIFO_CONFIG_0 register as
well as END_CLR, NAK_CLR and ARB_CLR of the STS register are marked as
w1c in the reference manual so there should be no need for
read-modify-write cycle.
@Hunter1753 Hunter1753 merged commit 1fa5918 into Hunter1753:bl808/i2c Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants