[U-Boot] [PATCH v4 04/34] musb: sunxi: Add fifo config

[Marek Vasut <marex@denx.de>]

On 02/11/2018 11:59 AM, Jagan Teki wrote:
> On Tue, Feb 6, 2018 at 8:11 PM, Marek Vasut <marex@denx.de> wrote:
>> On 02/06/2018 03:25 PM, Jagan Teki wrote:
>>> Unlike other Allwinner SOC's H3/H5/V3s OTG support 4 endpoints
>>> with relevant fifo configs, rest all have 5 endpoints.
>>> So add the fifo configs and defer them based on udevice_id compatible.
>>>
>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>> ---
>>>  drivers/usb/musb-new/sunxi.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 43 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
>>> index 0b7ff9f..562d311 100644
>>> --- a/drivers/usb/musb-new/sunxi.c
>>> +++ b/drivers/usb/musb-new/sunxi.c
>>> @@ -301,13 +301,52 @@ static const struct musb_platform_ops sunxi_musb_ops = {
>>>  #define SUNXI_MUSB_MAX_EP_NUM                6
>>>  #define SUNXI_MUSB_RAM_BITS          11
>>>
>>> +static struct musb_fifo_cfg sunxi_musb_mode_cfg[] = {
>>> +     MUSB_EP_FIFO_SINGLE(1, FIFO_TX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(1, FIFO_RX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(2, FIFO_TX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(2, FIFO_RX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(3, FIFO_TX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(3, FIFO_RX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(4, FIFO_TX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(4, FIFO_RX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(5, FIFO_TX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(5, FIFO_RX, 512),
>>
>> Generate all this programatically with a loop ?
>>
>>> +};
>>> +
>>> +/* H3/V3s OTG supports only 4 endpoints */
>>> +#define SUNXI_MUSB_MAX_EP_NUM_H3     5
>>> +
>>> +static struct musb_fifo_cfg sunxi_musb_mode_cfg_h3[] = {
>>> +     MUSB_EP_FIFO_SINGLE(1, FIFO_TX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(1, FIFO_RX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(2, FIFO_TX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(2, FIFO_RX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(3, FIFO_TX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(3, FIFO_RX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(4, FIFO_TX, 512),
>>> +     MUSB_EP_FIFO_SINGLE(4, FIFO_RX, 512),
>>> +};
>>> +
>>>  static struct musb_hdrc_config musb_config = {
>>> +     .fifo_cfg       = sunxi_musb_mode_cfg,
>>> +     .fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg),
>>>       .multipoint     = true,
>>>       .dyn_fifo       = true,
>>>       .num_eps        = SUNXI_MUSB_MAX_EP_NUM,
>>>       .ram_bits       = SUNXI_MUSB_RAM_BITS,
>>>  };
>>>
>>> +static struct musb_hdrc_config musb_config_h3 = {
>>> +     .fifo_cfg       = sunxi_musb_mode_cfg_h3,
>>> +     .fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_h3),
>>> +     .multipoint     = true,
>>> +     .dyn_fifo       = true,
>>> +     .soft_con       = true,
>>> +     .num_eps        = SUNXI_MUSB_MAX_EP_NUM_H3,
>>> +     .ram_bits       = SUNXI_MUSB_RAM_BITS,
>>> +};
>>> +
>>>  static int musb_usb_probe(struct udevice *dev)
>>>  {
>>>       struct sunxi_glue *glue = dev_get_priv(dev);
>>> @@ -328,7 +367,10 @@ static int musb_usb_probe(struct udevice *dev)
>>>
>>>       pdata.power = 250;
>>>       pdata.platform_ops = &sunxi_musb_ops;
>>> -     pdata.config = &musb_config;
>>> +     if (!device_is_compatible(dev, "allwinner,sun8i-h3-musb"))
>>
>> Instead of this, use the compatible's .data to pass the
>> musb_config(_h3). Then you can do
>> pdata.config = dev_get_driver_data(dev);
> 
> We've only diff with H3 compared to all other SOC's

_for_ _now_ . You cannot predict what AW will roll out next.

> and this make
> assigning same musb_config to non-H3 platforms, which will increasing
> some bytes(as of now 16bytes we have a10,a33, a31 and h3). compatible
> check here look suitable, I think.

The driver data are in the U-Boot binary anyway, just not used and thus
set to 0 . By adding this check, you're actually growing the size.

The driver data are there for this exact purpose -- associating metadata
with compatible strings, metadata which are to be used by the driver (ie
in probe() function). What you're doing here with the compatible check
is wrong.

-- 
Best regards,
Marek Vasut