[U-Boot] [PATCH v3 3/3] spi: mxs_spi: DM conversion

Marek Vasut marek.vasut at gmail.com
Fri May 11 11:31:49 UTC 2018


On 05/11/2018 01:09 PM, Akash Gajjar wrote:
> Hello Marek,
> 
> On Fri, May 11, 2018 at 4:09 PM, Marek Vasut <marek.vasut at gmail.com
> <mailto:marek.vasut at gmail.com>> wrote:
> 
>     On 05/11/2018 12:08 PM, Gajjar Akash wrote:
>     > Hi Marek,
>     >
>     > Thanks for the review comments.
>     >
>     >     > -struct mxs_spi_slave {
>     >     > -     struct spi_slave        slave;
>     >     > -     uint32_t                max_khz;
>     >     > -     uint32_t                mode;
>     >     > -     struct mxs_ssp_regs     *regs;
>     >     > +struct mxs_spi_priv {
>     >     > +     struct mxs_ssp_regs *regs;
>     >     > +     u32     max_khz;
>     >     > +     u32     mode;
>     >     > +     u32     bus;
>     >     > +     u32     cs;
>     >
>     >     Type cleanup should be a separate patch
>     >
>     >  
>     > Okay, I will prepare seperate patch for type cleanup.
>     >
>     >
>     >     >  };
>     >     >               if (mxs_wait_mask_set(&ssp_regs->hw_ssp_ctrl0_reg,
>     >     >                       SSP_CTRL0_RUN, MXS_SPI_MAX_TIMEOUT)) {
>     >     > -                     printf("MXS SPI: Timeout waiting for
>     start\n");
>     >     > +                     debug("MXS SPI: Timeout waiting for
>     start\n");
>     >
>     >     printf , we don't want to hide errors
>     >
>     >  
>     > okay, will revert it back to printf.
>     >
>     >
>     >     >                       return -ETIMEDOUT;
>     >     >               }
>     >
>     >     > +
>     >     > +#ifndef __SPI_MXS_H
>     >     > +#define __SPI_MXS_H
>     >     > +
>     >     > +struct mxs_spi_platdata {
>     >     > +     struct mxs_ssp_regs *regs;
>     >     > +     u32 bus;
>     >     > +     u32 max_hz;
>     >     > +     u32 cs;
>     >
>     >     Why is this header here at all ?
>     >
>     >  
>     > I didnt get this comment. do I need to place it somewhere else?
> 
>     See the beginning of this email, it seems the same structure exists
>     twice.
> 
> 
> ​My intention was to have two individual structure for private and
> platform data.​
> But now I could use one structre and access its members using two
> structure variables(one for private and one for platadata).
> 
> Is That looks okay?

I do not quite understand what you mean, but there is a duplication of
information here. That's a problem and should be fixed.

-- 
Best regards,
Marek Vasut


More information about the U-Boot mailing list