Hi Alagu,
On Thu, Oct 05, 2017 at 11:03:12PM +0530, Alagu Sankar wrote:
> Hi Gary,
>
>
> On 2017-10-05 15:39, Gary Bisson wrote:
> > Hi Alagu,
> >
> > On Sat, Sep 30, 2017 at 11:07:45PM +0530, silexcommon@xxxxxxxxx wrote:
> > > From: Alagu Sankar <alagusankar@xxxxxxxxxxxxxxx>
> > >
> > > convert different read write functions in sdio hif to bring it under a
> > > single read-write path. This helps in having a common dma bounce
> > > buffer
> > > implementation. Also helps in address modification that is required
> > > specific to change in certain mbox addresses of sdio_write.
> > >
> > > Signed-off-by: Alagu Sankar <alagusankar@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/net/wireless/ath/ath10k/sdio.c | 131
> > > ++++++++++++++++-----------------
> > > 1 file changed, 64 insertions(+), 67 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/ath/ath10k/sdio.c
> > > b/drivers/net/wireless/ath/ath10k/sdio.c
> > > index 77d4fa4..bb6fa67 100644
> > > --- a/drivers/net/wireless/ath/ath10k/sdio.c
> > > +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> > > @@ -36,6 +36,11 @@
> > >
> > > #define ATH10K_SDIO_DMA_BUF_SIZE (32 * 1024)
> > >
> > > +static int ath10k_sdio_read(struct ath10k *ar, u32 addr, void *buf,
> > > + u32 len, bool incr);
> > > +static int ath10k_sdio_write(struct ath10k *ar, u32 addr, const
> > > void *buf,
> > > + u32 len, bool incr);
> > > +
> >
> > As mentioned by Kalle, u32 needs to be size_t.
> Yes, the compiler I used is probably a step older and did not catch this.
> >
> > > /* inlined helper functions */
> > >
> > > /* Macro to check if DMA buffer is WORD-aligned and DMA-able.
> > > @@ -152,6 +157,7 @@ static int ath10k_sdio_config(struct ath10k *ar)
> > > struct sdio_func *func = ar_sdio->func;
> > > unsigned char byte, asyncintdelay = 2;
> > > int ret;
> > > + u32 addr;
> > >
> > > ath10k_dbg(ar, ATH10K_DBG_BOOT, "sdio configuration\n");
> > >
> > > @@ -180,9 +186,8 @@ static int ath10k_sdio_config(struct ath10k *ar)
> > > CCCR_SDIO_DRIVER_STRENGTH_ENABLE_C |
> > > CCCR_SDIO_DRIVER_STRENGTH_ENABLE_D);
> > >
> > > - ret = ath10k_sdio_func0_cmd52_wr_byte(func->card,
> > > - CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
> > > - byte);
> > > + addr = CCCR_SDIO_DRIVER_STRENGTH_ENABLE_ADDR,
> > > + ret = ath10k_sdio_func0_cmd52_wr_byte(func->card, addr, byte);
> >
> > Not sure this part is needed.
> This is to overcome checkpatch warning. Too big a names for the function and
> macro
> not helping in there. Will have to move it as a separate patch.
> >
> > > if (ret) {
> > > ath10k_warn(ar, "failed to enable driver strength: %d\n", ret);
> > > goto out;
> > > @@ -233,13 +238,16 @@ static int ath10k_sdio_config(struct ath10k *ar)
> > >
> > > static int ath10k_sdio_write32(struct ath10k *ar, u32 addr, u32 val)
> > > {
> > > - struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
> > > - struct sdio_func *func = ar_sdio->func;
> > > + __le32 *buf;
> > > int ret;
> > >
> > > - sdio_claim_host(func);
> > > + buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> > > + if (!buf)
> > > + return -ENOMEM;
> > >
> > > - sdio_writel(func, val, addr, &ret);
> > > + *buf = cpu_to_le32(val);
> > > +
> > > + ret = ath10k_sdio_write(ar, addr, &val, sizeof(val), true);
> >
> > Shouldn't we use buf instead of val? buf seems pretty useless otherwise.
> Yes, thanks for pointing this out. will be corrected in v2.
Do you have any timeframe on the v2?
Regards,
Gary
