[U-Boot] [PATCH 04/16] EXYNOS5: FDT : Decode peripheral id

[Simon Glass]

Hi,

On Wed, Dec 26, 2012 at 4:00 AM, Rajeshwari Birje
<rajeshwari.birje at gmail.com> wrote:
> Hi Minkyu Kang,
>
> Thank you for comments
>
> On Wed, Dec 26, 2012 at 5:24 PM, Minkyu Kang <mk7.kang at samsung.com> wrote:
>
>> Dear Rajeshwari Shinde,
>>
>> On 14/12/12 20:56, Rajeshwari Shinde wrote:
>> > Api is added to decode peripheral id based on the interrupt number
>> > of the peripheral.
>> >
>> > Signed-off-by: Rajeshwari Shinde <rajeshwari.s at samsung.com>
>> > Acked-by; Simon Glass <sjg at chromium.org>
>> > ---
>> > Changes in V1:
>> >       -Rebased on latest u-boot-samsung
>> >  arch/arm/cpu/armv7/exynos/pinmux.c        |   28
>> ++++++++++++++++++++++++++
>> >  arch/arm/include/asm/arch-exynos/periph.h |   31
>> ++++++++++++++++------------
>> >  arch/arm/include/asm/arch-exynos/pinmux.h |    8 +++++++
>> >  3 files changed, 54 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/arch/arm/cpu/armv7/exynos/pinmux.c
>> b/arch/arm/cpu/armv7/exynos/pinmux.c
>> > index f02f441..f9f6911 100644
>> > --- a/arch/arm/cpu/armv7/exynos/pinmux.c
>> > +++ b/arch/arm/cpu/armv7/exynos/pinmux.c
>> > @@ -22,6 +22,7 @@
>> >   */
>> >
>> >  #include <common.h>
>> > +#include <fdtdec.h>
>> >  #include <asm/arch/gpio.h>
>> >  #include <asm/arch/pinmux.h>
>> >  #include <asm/arch/sromc.h>
>> > @@ -396,3 +397,30 @@ int exynos_pinmux_config(int peripheral, int flags)
>> >               return -1;
>> >       }
>> >  }
>> > +
>> > +#ifdef CONFIG_OF_CONTROL
>> > +static int exynos5_pinmux_decode_periph_id(const void *blob, int node)
>> > +{
>> > +     int err;
>> > +     u32 cell[3];
>> > +
>> > +     err = fdtdec_get_int_array(blob, node, "interrupts", cell,
>> > +                                     ARRAY_SIZE(cell));
>> > +     if (err)
>> > +             return PERIPH_ID_NONE;
>> > +
>> > +     if ((131 > cell[1]) || (cell[1] < 31))
>>
>> What means 131 and 31?
>>
> will correct and remove magic numbers.
> They were the high and lowest interrupt numbers.
>
>>
>> > +             return cell[1];
>> > +
>> > +     debug(" invalid peripheral id\n");
>> > +     return PERIPH_ID_NONE;
>> > +}
>> > +
>> > +int pinmux_decode_periph_id(const void *blob, int node)
>> > +{
>> > +     if (cpu_is_exynos5())
>> > +             return  exynos5_pinmux_decode_periph_id(blob, node);
>> > +     else
>> > +             return PERIPH_ID_NONE;
>> > +}
>> > +#endif
>> > diff --git a/arch/arm/include/asm/arch-exynos/periph.h
>> b/arch/arm/include/asm/arch-exynos/periph.h
>> > index 13abd2d..783b77c 100644
>> > --- a/arch/arm/include/asm/arch-exynos/periph.h
>> > +++ b/arch/arm/include/asm/arch-exynos/periph.h
>> > @@ -25,12 +25,17 @@
>> >  #define __ASM_ARM_ARCH_PERIPH_H
>> >
>> >  /*
>> > - * Peripherals requiring clock/pinmux configuration. List will
>> > + * Peripherals requiring pinmux configuration0. List will
>>
>> configuration0?
>>
> - Will correct this
>
>>
>> >   * grow with support for more devices getting added.
>> > + * Numbering based on interrupt table.
>> >   *
>> >   */
>> >  enum periph_id {
>> > -     PERIPH_ID_I2C0,
>> > +     PERIPH_ID_UART0 = 51,
>> > +     PERIPH_ID_UART1,
>> > +     PERIPH_ID_UART2,
>> > +     PERIPH_ID_UART3,
>> > +     PERIPH_ID_I2C0 = 56,
>> >       PERIPH_ID_I2C1,
>> >       PERIPH_ID_I2C2,
>> >       PERIPH_ID_I2C3,
>> > @@ -38,22 +43,22 @@ enum periph_id {
>> >       PERIPH_ID_I2C5,
>> >       PERIPH_ID_I2C6,
>> >       PERIPH_ID_I2C7,
>> > -     PERIPH_ID_I2S1,
>> > -     PERIPH_ID_SDMMC0,
>> > +     PERIPH_ID_SPI0 = 68,
>> > +     PERIPH_ID_SPI1,
>> > +     PERIPH_ID_SPI2,
>> > +     PERIPH_ID_SDMMC0 = 75,
>> >       PERIPH_ID_SDMMC1,
>> >       PERIPH_ID_SDMMC2,
>> >       PERIPH_ID_SDMMC3,
>> > -     PERIPH_ID_SDMMC4,
>> > -     PERIPH_ID_SROMC,
>> > -     PERIPH_ID_SPI0,
>> > -     PERIPH_ID_SPI1,
>> > -     PERIPH_ID_SPI2,
>> > +     PERIPH_ID_I2S1 = 99,
>> > +
>> > +/* Since following peripherals do not have shared peripheral interrupts
>> (SPIs)
>>
>> Please check multi line comment rule.
>>
> - Will correct this
>
>>
>> > + * they are numbered arbitiraly after the maximum SPIs Exynos has (128)
>> > + */
>> > +     PERIPH_ID_SROMC = 128,
>> >       PERIPH_ID_SPI3,
>> >       PERIPH_ID_SPI4,
>> > -     PERIPH_ID_UART0,
>> > -     PERIPH_ID_UART1,
>> > -     PERIPH_ID_UART2,
>> > -     PERIPH_ID_UART3,
>> > +     PERIPH_ID_SDMMC4,
>> >
>> >       PERIPH_ID_COUNT,
>> >       PERIPH_ID_NONE = -1,
>> > diff --git a/arch/arm/include/asm/arch-exynos/pinmux.h
>> b/arch/arm/include/asm/arch-exynos/pinmux.h
>> > index 10ea736..00cbb0d 100644
>> > --- a/arch/arm/include/asm/arch-exynos/pinmux.h
>> > +++ b/arch/arm/include/asm/arch-exynos/pinmux.h
>> > @@ -55,4 +55,12 @@ enum {
>> >   */
>> >  int exynos_pinmux_config(int peripheral, int flags);
>> >
>> > +/**
>>
>> please remove "*".
>>
> - Will correct this

The extra * indicates a comment with doxygen / DocBook comments, so I
think we should keep this if we can. There is a bit of a push in
U-Boot to use DocBook now (like include/cbfs.h). The format is
slightly different from doxygen but it's probably putting to run a
converter to switch over at one point.

>
>>
>> > + * Decode the peripheral id using the interrpt numbers.
>> > + *
>> > + * @param blob  Device tree blbo
>> > + * @param node  FDT I2C node to find
>> > + * @return peripheral id if ok, PERIPH_ID_NONE on error
>> > + */
>> > +int pinmux_decode_periph_id(const void *blob, int node);
>> >  #endif
>> >

Regards,
Simon