Subject: Re: [PATCH 1/1] net: dsa: replacing the hard-coded sized array "dsa_switch" by dynamic one

From: Andrey Volkov <[email-masked]>
Date: Thu, 04 Dec 2014 18:29:55 +0100

Le 02/12/2014 21:40, Florian Fainelli a écrit :
> On 02/12/14 06:50, Andrey Volkov wrote:
>> Hello,
>>
>> In time of developing one of our devices (with huge, more then 6, number of onboard switches),
>> I've bumped with this ancient, I hope, restriction in the 'struct dsa_switch_tree' definition. 
>> So this simple patch remove this restriction and make dsa_switch_tree more scalable for 
>> the "usual" 1-2 switches configuration too.
> Sounds reasonable to me, you probably want to resubmit and trim the
> "Hello" form your commit message.
>
>> P.S. I've plans to fix hardcoded number of ports too, but it is not so easy as with number of switches.
>> So if someone have any objections/suggestions I'll happy to discuss them.
> I think the number of ports in a switch is something that should come
> from the switch driver, and eventually intersected with what the
> platform configuration has provided.
Yes it's exactly what I've in my mind. Also in our project I need to handle currently unsupported case:
when switches combined into the stacks by more than one hardwired links, i.e. some ports must be
configured as part of 'hard-coded' trunks.

> The difficulty is in case of sparse port number allocation because you
> still want to allocate e.g: 6 ports even though Port 0 and 5 are used, I
> don't think we want to introduce a logical to physical mapping, that
> would be too error prone.
Agree.

>
>> Signed-off-by: Andrey Volkov <[email-masked]>                                                                                                                                                                                    
>> ---                                                                                                                                                                                                                                          
>>  include/net/dsa.h |    3 +--                                                                                                                                                                                                                
>>  net/dsa/dsa.c     |    7 +++----                                                                                                                                                                                                            
>>  2 files changed, 4 insertions(+), 6 deletions(-)                                                                                                                                                                                            
>>                                                                                                                                                                                                                                              
>> diff --git a/include/net/dsa.h b/include/net/dsa.h                                                                                                                                                                                           
>> index ed3c34b..733db2e 100644                                                                                                                                                                                                                
>> --- a/include/net/dsa.h
>> +++ b/include/net/dsa.h
>> @@ -28,7 +28,6 @@ enum dsa_tag_protocol {
>>         DSA_TAG_PROTO_BRCM,
>>  };
>>  
>> -#define DSA_MAX_SWITCHES       4
>>  #define DSA_MAX_PORTS          12
>>  
>>  struct dsa_chip_data {
>> @@ -117,7 +116,7 @@ struct dsa_switch_tree {
>>         /*
>>          * Data for the individual switch chips.
>>          */
>> -       struct dsa_switch       *ds[DSA_MAX_SWITCHES];
>> +       struct dsa_switch       *ds[];
>>  };
>>  
>>  struct dsa_switch {
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index 322c778..c081a19 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -604,8 +604,6 @@ static int dsa_of_probe(struct platform_device *pdev)
>>         pdev->dev.platform_data = pd;
>>         pd->netdev = &ethernet_dev->dev;
>>         pd->nr_chips = of_get_child_count(np);
>> -       if (pd->nr_chips > DSA_MAX_SWITCHES)
>> -               pd->nr_chips = DSA_MAX_SWITCHES;
>>  
>>         pd->chip = kcalloc(pd->nr_chips, sizeof(struct dsa_chip_data),
>>                            GFP_KERNEL);
>> @@ -717,7 +715,7 @@ static int dsa_probe(struct platform_device *pdev)
>>                 pd = pdev->dev.platform_data;
>>         }
>>  
>> -       if (pd == NULL || pd->netdev == NULL)
>> +       if (pd == NULL || pd->netdev == NULL || pd->nr_chips == 0)
>>                 return -EINVAL;
>>  
>>         dev = dev_to_net_device(pd->netdev);
>> @@ -732,7 +730,8 @@ static int dsa_probe(struct platform_device *pdev)
>>                 goto out;
>>         }
>>  
>> -       dst = kzalloc(sizeof(*dst), GFP_KERNEL);
>> +       dst = kzalloc(sizeof(*dst) +
>> +                       sizeof(struct dsa_switch *) * pd->nr_chips, GFP_KERNEL);
>>         if (dst == NULL) {
>>                 dev_put(dev);
>>                 ret = -ENOMEM;
>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email-masked]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html