Hi Ian,

So it took me a while to get to this patch series. It is indeed way to big and not well structured making it difficult to review the patches on a per-patch basis.

I decided to limit myself to looking at the patches involving bcmsdh.c doing a 'git log bcmsdh.c' on the applied patches. Still twenty patches to review. I reviewed the first 15 patches of the series and skipped patch [13/34] as it did not touch bcmsdh.c. A few response already slipped to the mailing list so I will post the rest shortly. Here is the first one.

On 26-07-17 22:25, Ian Molton wrote:
        All the other IO functions are the other way round in this
driver. Make this one match.

Core SDIO functions are indeed the other way around, but the IO
functions in this file use (addr, data) pattern. So should we aim to get it all consistent with core SDIO or just consistent on its own.

Signed-off-by: Ian Molton <i...@mnementh.co.uk>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 984c1d0560b1..f585dfd89453 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -230,8 +230,8 @@ void brcmf_sdiod_change_state(struct brcmf_sdio_dev 
*sdiodev,
        sdiodev->state = state;
 }

-static inline int brcmf_sdiod_f0_writeb(struct sdio_func *func,
-                                       uint regaddr, u8 byte)
+static inline int brcmf_sdiod_f0_writeb(struct sdio_func *func, u8 byte,
+                                       uint regaddr)

The second line needs to be aligned on same column as the opening bracket.

Regards,
Arend

Reply via email to