Re: [PATCH] MIPS: Octeon: Add Octeon III CN7XXX interface detection

[Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>]

Hi,

Thanks for the review. Comments below.

On 02/02/16 11:38, Sergei Shtylyov wrote:
> Hello.
>
> On 2/2/2016 2:25 PM, Zubair Lutfullah Kakakhel wrote:
>
>> Add basic CN7XXX interface detection.
>>
>> This allows the kernel to boot with ethernet working as it initializes
>> the ethernet ports with SGMII instead of defaulting to RGMII routines.
>>
>> Tested on the utm8 from Rhino Labs with a CN7130.
>>
>> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>> ---
>>   arch/mips/cavium-octeon/executive/cvmx-helper.c | 41 +++++++++++++++++++++++++
>>   1 file changed, 41 insertions(+)
>>
>> diff --git a/arch/mips/cavium-octeon/executive/cvmx-helper.c b/arch/mips/cavium-octeon/executive/cvmx-helper.c
>> index 376701f..1a28009 100644
>> --- a/arch/mips/cavium-octeon/executive/cvmx-helper.c
>> +++ b/arch/mips/cavium-octeon/executive/cvmx-helper.c
> [...]
>> @@ -260,6 +262,39 @@ static cvmx_helper_interface_mode_t __cvmx_get_mode_octeon2(int interface)
>>   }
>>
>>   /**
>> + * @INTERNAL
>> + * Return interface mode for CN7XXX.
>> + */
>> +static cvmx_helper_interface_mode_t __cvmx_get_mode_cn7xxx(int interface)
>
>     Not *unsigned*?

The rest of the instances in the file don't have unsigned.

Probably because it is an enum..

>
>> +{
>> +    union cvmx_gmxx_inf_mode mode;
>> +
>> +    mode.u64 = cvmx_read_csr(CVMX_GMXX_INF_MODE(interface));
>> +
>> +    if (interface < 2) {        /* SGMII/QSGMII/XAUI */
>> +        switch (mode.cn68xx.mode) {
>> +        case 0:
>> +            return CVMX_HELPER_INTERFACE_MODE_DISABLED;
>> +        case 1:
>> +        case 2:
>> +            return CVMX_HELPER_INTERFACE_MODE_SGMII;
>> +        case 3:
>> +            return CVMX_HELPER_INTERFACE_MODE_XAUI;
>> +        default:
>> +            return CVMX_HELPER_INTERFACE_MODE_SGMII;
>> +        }
>> +    } else if (interface == 2)    /* NPI */
>> +        return CVMX_HELPER_INTERFACE_MODE_NPI;
>> +    else if (interface == 3)    /* LOOP */
>> +        return CVMX_HELPER_INTERFACE_MODE_LOOP;
>> +    else if (interface == 4)    /* RGMII (AGL) */
>> +        return CVMX_HELPER_INTERFACE_MODE_RGMII;
>
>     This is asking to be a *switch* statement.

Ask and you shall receive

(⌐▀͡ ̯ʖ▀)

Thanks
ZubairLK

>
>> +
>> +    return CVMX_HELPER_INTERFACE_MODE_DISABLED;
>> +}
>> +
>> +
>> +/**
>>    * Get the operating mode of an interface. Depending on the Octeon
>>    * chip and configuration, this function returns an enumeration
>>    * of the type of packet I/O supported by an interface.
> [...]
>
> MBR, Sergei
>
>