RE: [PATCH] ARM: OMAP3+: am33xx id: Add new am33xx specific function to check dev_feature

2013-05-17 Thread Hiremath, Vaibhav

 -Original Message-
 From: Kevin Hilman [mailto:khil...@linaro.org]
 Sent: Tuesday, May 14, 2013 3:23 AM
 To: Hiremath, Vaibhav
 Cc: linux-omap@vger.kernel.org; t...@atomide.com; linux-arm-
 ker...@lists.infradead.org
 Subject: Re: [PATCH] ARM: OMAP3+: am33xx id: Add new am33xx specific
 function to check dev_feature
 
 Vaibhav Hiremath hvaib...@ti.com writes:
 
  Layout of DEV_FEATURE register (offset = 0x604) is different
  between TI81xx and AM33xx device, so create separate function
  which will check for features available on specific AM33xx SoC
  and set the flags accordingly.
 
  Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
 
 Minor nit below, otherwise...
 
 Reviewed-by: Kevin Hilman khil...@linaro.org
 
  ---
   arch/arm/mach-omap2/control.h |5 +
   arch/arm/mach-omap2/id.c  |   13 +
   arch/arm/mach-omap2/io.c  |2 +-
   arch/arm/mach-omap2/soc.h |1 +
   4 files changed, 20 insertions(+), 1 deletions(-)
 
  diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-
 omap2/control.h
  index e6c3281..4acdfc5 100644
  --- a/arch/arm/mach-omap2/control.h
  +++ b/arch/arm/mach-omap2/control.h
  @@ -358,6 +358,11 @@
   #define AM33XX_CONTROL_STATUS_SYSBOOT1_WIDTH   0x2
   #define AM33XX_CONTROL_STATUS_SYSBOOT1_MASK(0x3  22)
 
  +/* DEV Feature register to identify AM33XX features */
  +#define AM33XX_DEV_FEATURE 0x604
  +#define AM33XX_SGX_SHIFT   29
 
 You don't need the shift value anywhere in the code, so...
 
  +#define AM33XX_SGX_MASK(1  AM33XX_SGX_SHIFT)
 
 #define AM33XX_SGX_MASK   BIT(29)
 
 instead?
 
 Otherwise, rest of patch looks fine.
 
Thanks for the review Kevin.
I just sent out V2 version of the patch with your reviewed-by
With changes you mentioned.

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: OMAP3+: am33xx id: Add new am33xx specific function to check dev_feature

2013-05-13 Thread Kevin Hilman
Vaibhav Hiremath hvaib...@ti.com writes:

 Layout of DEV_FEATURE register (offset = 0x604) is different
 between TI81xx and AM33xx device, so create separate function
 which will check for features available on specific AM33xx SoC
 and set the flags accordingly.

 Signed-off-by: Vaibhav Hiremath hvaib...@ti.com

Minor nit below, otherwise...

Reviewed-by: Kevin Hilman khil...@linaro.org

 ---
  arch/arm/mach-omap2/control.h |5 +
  arch/arm/mach-omap2/id.c  |   13 +
  arch/arm/mach-omap2/io.c  |2 +-
  arch/arm/mach-omap2/soc.h |1 +
  4 files changed, 20 insertions(+), 1 deletions(-)

 diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
 index e6c3281..4acdfc5 100644
 --- a/arch/arm/mach-omap2/control.h
 +++ b/arch/arm/mach-omap2/control.h
 @@ -358,6 +358,11 @@
  #define AM33XX_CONTROL_STATUS_SYSBOOT1_WIDTH 0x2
  #define AM33XX_CONTROL_STATUS_SYSBOOT1_MASK  (0x3  22)
  
 +/* DEV Feature register to identify AM33XX features */
 +#define AM33XX_DEV_FEATURE   0x604
 +#define AM33XX_SGX_SHIFT 29

You don't need the shift value anywhere in the code, so...

 +#define AM33XX_SGX_MASK  (1  AM33XX_SGX_SHIFT)

#define AM33XX_SGX_MASK BIT(29)

instead?

Otherwise, rest of patch looks fine.

Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html