Re: [edk2] [platforms: PATCH 02/12] Marvell/Library: ArmadaBoardDescLib: Add GPIO information

2018-12-04 Thread Leif Lindholm
On Wed, Nov 14, 2018 at 01:12:22AM +, Leif Lindholm wrote:
> On Sat, Oct 20, 2018 at 03:57:31AM +0200, Marcin Wojtas wrote:
> > This patch extends library with GPIO devices per-board
> > description. Both embedded SoC controllers and
> > I2C IO expanders are supported. Add a helper routine
> > for obtaining information about the latter.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas 
> > ---
> >  Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h | 23 
> > 
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h 
> > b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> > index ee8e06e..109164c 100644
> > --- a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> > +++ b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> > @@ -25,6 +25,29 @@ typedef struct {
> >  } MV_BOARD_COMPHY_DESC;
> >  
> >  //
> > +// GPIO devices per-board description
> > +//
> > +typedef struct {
> > +  UINTN ChipId;
> > +  UINTN I2cAddress;
> > +  UINTN I2cBus;
> > +} MV_I2C_IO_EXPANDER_DESC;
> > +
> > +typedef struct {
> > +  MV_SOC_GPIO_DESC*SoC;
> > +  UINTNGpioDevCount;
> > +  MV_I2C_IO_EXPANDER_DESC *I2cIoExpanderDesc;
> > +  UINTNI2cIoExpanderCount;
> > +} MV_BOARD_GPIO_DESC;
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +ArmadaBoardDescGpioGet (
> > +  IN OUT MV_I2C_IO_EXPANDER_DESC **I2cIoExpanderDesc,
> > +  IN OUT UINTN*I2cIoExpanderCount
> 
> Please expand all DESC/Desc to descriptor/description/whatever as
> appropriate.
> With that, Reviewed-by: Leif Lindholm 

Actually, I spotted something in 3-4/12 on finally getting back to this
set. The function is called ArmadaBoardDescGpioGet, but the IN/OUT
parameters talk abour I2c.

This seems like either a lack of abstraction (which given this is a
generic library class seems suboptimal).

Could that too be addressed in v2? (I'll mention it here instead of in
the users.)

Regards,

Leif

> > +  );
> > +
> > +//
> >  // I2C devices per-board description
> >  //
> >  typedef struct {
> > -- 
> > 2.7.4
> > 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [platforms: PATCH 02/12] Marvell/Library: ArmadaBoardDescLib: Add GPIO information

2018-11-14 Thread Leif Lindholm
On Wed, Nov 14, 2018 at 07:16:33AM +0100, Marcin Wojtas wrote:
> Hi Leif,
> 
> śr., 14 lis 2018 o 02:12 Leif Lindholm  napisał(a):
> >
> > On Sat, Oct 20, 2018 at 03:57:31AM +0200, Marcin Wojtas wrote:
> > > This patch extends library with GPIO devices per-board
> > > description. Both embedded SoC controllers and
> > > I2C IO expanders are supported. Add a helper routine
> > > for obtaining information about the latter.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Marcin Wojtas 
> > > ---
> > >  Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h | 23 
> > > 
> > >  1 file changed, 23 insertions(+)
> > >
> > > diff --git a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h 
> > > b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> > > index ee8e06e..109164c 100644
> > > --- a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> > > +++ b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> > > @@ -25,6 +25,29 @@ typedef struct {
> > >  } MV_BOARD_COMPHY_DESC;
> > >
> > >  //
> > > +// GPIO devices per-board description
> > > +//
> > > +typedef struct {
> > > +  UINTN ChipId;
> > > +  UINTN I2cAddress;
> > > +  UINTN I2cBus;
> > > +} MV_I2C_IO_EXPANDER_DESC;
> > > +
> > > +typedef struct {
> > > +  MV_SOC_GPIO_DESC*SoC;
> > > +  UINTNGpioDevCount;
> > > +  MV_I2C_IO_EXPANDER_DESC *I2cIoExpanderDesc;
> > > +  UINTNI2cIoExpanderCount;
> > > +} MV_BOARD_GPIO_DESC;
> > > +
> > > +EFI_STATUS
> > > +EFIAPI
> > > +ArmadaBoardDescGpioGet (
> > > +  IN OUT MV_I2C_IO_EXPANDER_DESC **I2cIoExpanderDesc,
> > > +  IN OUT UINTN*I2cIoExpanderCount
> >
> > Please expand all DESC/Desc to descriptor/description/whatever as
> > appropriate.
> 
> Just to confirm - until now in every case there was an abbreviation
> used (in SoCDescLib, structures names, etc.). It may look somewhat
> inconsistently, but of course I can change according to your wish.
> Please let know your thoughts on that.

Well, I'd prefer to change it all, but I'm happy to stick to following
the coding style for new patches.

/
Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [platforms: PATCH 02/12] Marvell/Library: ArmadaBoardDescLib: Add GPIO information

2018-11-13 Thread Marcin Wojtas
Hi Leif,

śr., 14 lis 2018 o 02:12 Leif Lindholm  napisał(a):
>
> On Sat, Oct 20, 2018 at 03:57:31AM +0200, Marcin Wojtas wrote:
> > This patch extends library with GPIO devices per-board
> > description. Both embedded SoC controllers and
> > I2C IO expanders are supported. Add a helper routine
> > for obtaining information about the latter.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas 
> > ---
> >  Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h | 23 
> > 
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h 
> > b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> > index ee8e06e..109164c 100644
> > --- a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> > +++ b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> > @@ -25,6 +25,29 @@ typedef struct {
> >  } MV_BOARD_COMPHY_DESC;
> >
> >  //
> > +// GPIO devices per-board description
> > +//
> > +typedef struct {
> > +  UINTN ChipId;
> > +  UINTN I2cAddress;
> > +  UINTN I2cBus;
> > +} MV_I2C_IO_EXPANDER_DESC;
> > +
> > +typedef struct {
> > +  MV_SOC_GPIO_DESC*SoC;
> > +  UINTNGpioDevCount;
> > +  MV_I2C_IO_EXPANDER_DESC *I2cIoExpanderDesc;
> > +  UINTNI2cIoExpanderCount;
> > +} MV_BOARD_GPIO_DESC;
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +ArmadaBoardDescGpioGet (
> > +  IN OUT MV_I2C_IO_EXPANDER_DESC **I2cIoExpanderDesc,
> > +  IN OUT UINTN*I2cIoExpanderCount
>
> Please expand all DESC/Desc to descriptor/description/whatever as
> appropriate.

Just to confirm - until now in every case there was an abbreviation
used (in SoCDescLib, structures names, etc.). It may look somewhat
inconsistently, but of course I can change according to your wish.
Please let know your thoughts on that.

Thanks,
Marcin

> With that, Reviewed-by: Leif Lindholm 
>
> > +  );
> > +
> > +//
> >  // I2C devices per-board description
> >  //
> >  typedef struct {
> > --
> > 2.7.4
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [platforms: PATCH 02/12] Marvell/Library: ArmadaBoardDescLib: Add GPIO information

2018-11-13 Thread Leif Lindholm
On Sat, Oct 20, 2018 at 03:57:31AM +0200, Marcin Wojtas wrote:
> This patch extends library with GPIO devices per-board
> description. Both embedded SoC controllers and
> I2C IO expanders are supported. Add a helper routine
> for obtaining information about the latter.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h | 23 
> 
>  1 file changed, 23 insertions(+)
> 
> diff --git a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h 
> b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> index ee8e06e..109164c 100644
> --- a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> +++ b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
> @@ -25,6 +25,29 @@ typedef struct {
>  } MV_BOARD_COMPHY_DESC;
>  
>  //
> +// GPIO devices per-board description
> +//
> +typedef struct {
> +  UINTN ChipId;
> +  UINTN I2cAddress;
> +  UINTN I2cBus;
> +} MV_I2C_IO_EXPANDER_DESC;
> +
> +typedef struct {
> +  MV_SOC_GPIO_DESC*SoC;
> +  UINTNGpioDevCount;
> +  MV_I2C_IO_EXPANDER_DESC *I2cIoExpanderDesc;
> +  UINTNI2cIoExpanderCount;
> +} MV_BOARD_GPIO_DESC;
> +
> +EFI_STATUS
> +EFIAPI
> +ArmadaBoardDescGpioGet (
> +  IN OUT MV_I2C_IO_EXPANDER_DESC **I2cIoExpanderDesc,
> +  IN OUT UINTN*I2cIoExpanderCount

Please expand all DESC/Desc to descriptor/description/whatever as
appropriate.
With that, Reviewed-by: Leif Lindholm 

> +  );
> +
> +//
>  // I2C devices per-board description
>  //
>  typedef struct {
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [platforms: PATCH 02/12] Marvell/Library: ArmadaBoardDescLib: Add GPIO information

2018-10-19 Thread Marcin Wojtas
This patch extends library with GPIO devices per-board
description. Both embedded SoC controllers and
I2C IO expanders are supported. Add a helper routine
for obtaining information about the latter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h | 23 
 1 file changed, 23 insertions(+)

diff --git a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h 
b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
index ee8e06e..109164c 100644
--- a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
+++ b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
@@ -25,6 +25,29 @@ typedef struct {
 } MV_BOARD_COMPHY_DESC;
 
 //
+// GPIO devices per-board description
+//
+typedef struct {
+  UINTN ChipId;
+  UINTN I2cAddress;
+  UINTN I2cBus;
+} MV_I2C_IO_EXPANDER_DESC;
+
+typedef struct {
+  MV_SOC_GPIO_DESC*SoC;
+  UINTNGpioDevCount;
+  MV_I2C_IO_EXPANDER_DESC *I2cIoExpanderDesc;
+  UINTNI2cIoExpanderCount;
+} MV_BOARD_GPIO_DESC;
+
+EFI_STATUS
+EFIAPI
+ArmadaBoardDescGpioGet (
+  IN OUT MV_I2C_IO_EXPANDER_DESC **I2cIoExpanderDesc,
+  IN OUT UINTN*I2cIoExpanderCount
+  );
+
+//
 // I2C devices per-board description
 //
 typedef struct {
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel