RE: [PATCH v4] ARM: DaVinci: DM646x Video: Platform and board specific setup

2009-08-04 Thread chaithrika
Russell,

Requesting your ack on this patch.

Regards,
Chaithrika

On Wed, Aug 05, 2009 at 20:17:42, Chaithrika U S wrote:
> Platform specific display device setup for DM646x EVM
> 
> Add platform device and resource structures. Also define a platform
specific
> clock setup function that can be accessed by the driver to configure the
clock
> and CPLD.
> 
> Signed-off-by: Manjunath Hadli 
> Signed-off-by: Brijesh Jadav 
> Signed-off-by: Chaithrika U S 
> Signed-off-by: Kevin Hilman 
> ---
> Applies to Davinci GIT tree. Minor updates like change in structure name-
> subdev_info to vpif_subdev_info and correction to VDD3P3V_VID_MASK value.
> 
>  arch/arm/mach-davinci/board-dm646x-evm.c|  125
+++
>  arch/arm/mach-davinci/dm646x.c  |   62 +
>  arch/arm/mach-davinci/include/mach/dm646x.h |   24 +
>  3 files changed, 211 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c
b/arch/arm/mach-davinci/board-dm646x-evm.c
> index b1bf18c..8c88fd0 100644
> --- a/arch/arm/mach-davinci/board-dm646x-evm.c
> +++ b/arch/arm/mach-davinci/board-dm646x-evm.c
> @@ -63,6 +63,19 @@
>  #define DM646X_EVM_PHY_MASK  (0x2)
>  #define DM646X_EVM_MDIO_FREQUENCY(220) /* PHY bus frequency */
>  
> +#define VIDCLKCTL_OFFSET (0x38)
> +#define VSCLKDIS_OFFSET  (0x6c)
> +
> +#define VCH2CLK_MASK (BIT_MASK(10) | BIT_MASK(9) | BIT_MASK(8))
> +#define VCH2CLK_SYSCLK8  (BIT(9))
> +#define VCH2CLK_AUXCLK   (BIT(9) | BIT(8))
> +#define VCH3CLK_MASK (BIT_MASK(14) | BIT_MASK(13) | BIT_MASK(12))
> +#define VCH3CLK_SYSCLK8  (BIT(13))
> +#define VCH3CLK_AUXCLK   (BIT(14) | BIT(13))
> +
> +#define VIDCH2CLK(BIT(10))
> +#define VIDCH3CLK(BIT(11))
> +
>  static struct davinci_uart_config uart_config __initdata = {
>   .enabled_uarts = (1 << 0),
>  };
> @@ -288,6 +301,40 @@ static struct snd_platform_data dm646x_evm_snd_data[]
= {
>   },
>  };
>  
> +static struct i2c_client *cpld_client;
> +
> +static int cpld_video_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + cpld_client = client;
> + return 0;
> +}
> +
> +static int __devexit cpld_video_remove(struct i2c_client *client)
> +{
> + cpld_client = NULL;
> + return 0;
> +}
> +
> +static const struct i2c_device_id cpld_video_id[] = {
> + { "cpld_video", 0 },
> + { }
> +};
> +
> +static struct i2c_driver cpld_video_driver = {
> + .driver = {
> + .name   = "cpld_video",
> + },
> + .probe  = cpld_video_probe,
> + .remove = cpld_video_remove,
> + .id_table   = cpld_video_id,
> +};
> +
> +static void evm_init_cpld(void)
> +{
> + i2c_add_driver(&cpld_video_driver);
> +}
> +
>  static struct i2c_board_info __initdata i2c_info[] =  {
>   {
>   I2C_BOARD_INFO("24c256", 0x50),
> @@ -300,6 +347,9 @@ static struct i2c_board_info __initdata i2c_info[] =
{
>   {
>   I2C_BOARD_INFO("cpld_reg0", 0x3a),
>   },
> + {
> + I2C_BOARD_INFO("cpld_video", 0x3B),
> + },
>  };
>  
>  static struct davinci_i2c_platform_data i2c_pdata = {
> @@ -307,11 +357,85 @@ static struct davinci_i2c_platform_data i2c_pdata =
{
>   .bus_delay  = 0 /* usec */,
>  };
>  
> +static int set_vpif_clock(int mux_mode, int hd)
> +{
> + int val = 0;
> + int err = 0;
> + unsigned int value;
> + void __iomem *base = IO_ADDRESS(DAVINCI_SYSTEM_MODULE_BASE);
> +
> + if (!cpld_client)
> + return -ENXIO;
> +
> + /* disable the clock */
> + value = __raw_readl(base + VSCLKDIS_OFFSET);
> + value |= (VIDCH3CLK | VIDCH2CLK);
> + __raw_writel(value, base + VSCLKDIS_OFFSET);
> +
> + val = i2c_smbus_read_byte(cpld_client);
> + if (val < 0)
> + return val;
> +
> + if (mux_mode == 1)
> + val &= ~0x40;
> + else
> + val |= 0x40;
> +
> + err = i2c_smbus_write_byte(cpld_client, val);
> + if (err)
> + return err;
> +
> + value = __raw_readl(base + VIDCLKCTL_OFFSET);
> + value &= ~(VCH2CLK_MASK);
> + value &= ~(VCH3CLK_MASK);
> +
> + if (hd >= 1)
> + value |= (VCH2CLK_SYSCLK8 | VCH3CLK_SYSCLK8);
> + else
> + value |= (VCH2CLK_AUXCLK | VCH3CLK_AUXCLK);
> +
> + __raw_writel(value, base + VIDCLKCTL_OFFSET);
> +
> + /* enable the clock */
> + value = __raw_readl(base + VSCLKDIS_OFFSET);
> + value &= ~(VIDCH3CLK | VIDCH2CLK);
> + __raw_writel(value, base + VSCLKDIS_OFFSET);
> +
> + return 0;
> +}
> +
> +static const struct vpif_subdev_info dm646x_vpif_subdev[] = {
> + {
> + .addr   = 0x2A,
> + .name   = "adv7343",
> + },
> + {
> + .addr   = 0x2C,
> + .name   = "ths7303",
> + },
> +};
> +
> +static const char *output[] = {
> + "

Re: [PATCH v4] ARM: DaVinci: DM646x Video: Platform and board specific setup

2009-08-07 Thread Kevin Hilman
"chaithrika"  writes:

> Russell,
>
> Requesting your ack on this patch.
>

Chaithrika, Mauro,

We don't need Russell's ack on this.  You've incorporated his comments
and my signoff is enough for stuff under arch/arm/mach-davinci/*

Mauro, go ahead and merge this.

Kevin


>
> On Wed, Aug 05, 2009 at 20:17:42, Chaithrika U S wrote:
>> Platform specific display device setup for DM646x EVM
>> 
>> Add platform device and resource structures. Also define a platform
> specific
>> clock setup function that can be accessed by the driver to configure the
> clock
>> and CPLD.
>> 
>> Signed-off-by: Manjunath Hadli 
>> Signed-off-by: Brijesh Jadav 
>> Signed-off-by: Chaithrika U S 
>> Signed-off-by: Kevin Hilman 
>> ---
>> Applies to Davinci GIT tree. Minor updates like change in structure name-
>> subdev_info to vpif_subdev_info and correction to VDD3P3V_VID_MASK value.
>> 
>>  arch/arm/mach-davinci/board-dm646x-evm.c|  125
> +++
>>  arch/arm/mach-davinci/dm646x.c  |   62 +
>>  arch/arm/mach-davinci/include/mach/dm646x.h |   24 +
>>  3 files changed, 211 insertions(+), 0 deletions(-)
>> 
>> diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c
> b/arch/arm/mach-davinci/board-dm646x-evm.c
>> index b1bf18c..8c88fd0 100644
>> --- a/arch/arm/mach-davinci/board-dm646x-evm.c
>> +++ b/arch/arm/mach-davinci/board-dm646x-evm.c
>> @@ -63,6 +63,19 @@
>>  #define DM646X_EVM_PHY_MASK (0x2)
>>  #define DM646X_EVM_MDIO_FREQUENCY   (220) /* PHY bus frequency */
>>  
>> +#define VIDCLKCTL_OFFSET(0x38)
>> +#define VSCLKDIS_OFFSET (0x6c)
>> +
>> +#define VCH2CLK_MASK(BIT_MASK(10) | BIT_MASK(9) | 
>> BIT_MASK(8))
>> +#define VCH2CLK_SYSCLK8 (BIT(9))
>> +#define VCH2CLK_AUXCLK  (BIT(9) | BIT(8))
>> +#define VCH3CLK_MASK(BIT_MASK(14) | BIT_MASK(13) | 
>> BIT_MASK(12))
>> +#define VCH3CLK_SYSCLK8 (BIT(13))
>> +#define VCH3CLK_AUXCLK  (BIT(14) | BIT(13))
>> +
>> +#define VIDCH2CLK   (BIT(10))
>> +#define VIDCH3CLK   (BIT(11))
>> +
>>  static struct davinci_uart_config uart_config __initdata = {
>>  .enabled_uarts = (1 << 0),
>>  };
>> @@ -288,6 +301,40 @@ static struct snd_platform_data dm646x_evm_snd_data[]
> = {
>>  },
>>  };
>>  
>> +static struct i2c_client *cpld_client;
>> +
>> +static int cpld_video_probe(struct i2c_client *client,
>> +const struct i2c_device_id *id)
>> +{
>> +cpld_client = client;
>> +return 0;
>> +}
>> +
>> +static int __devexit cpld_video_remove(struct i2c_client *client)
>> +{
>> +cpld_client = NULL;
>> +return 0;
>> +}
>> +
>> +static const struct i2c_device_id cpld_video_id[] = {
>> +{ "cpld_video", 0 },
>> +{ }
>> +};
>> +
>> +static struct i2c_driver cpld_video_driver = {
>> +.driver = {
>> +.name   = "cpld_video",
>> +},
>> +.probe  = cpld_video_probe,
>> +.remove = cpld_video_remove,
>> +.id_table   = cpld_video_id,
>> +};
>> +
>> +static void evm_init_cpld(void)
>> +{
>> +i2c_add_driver(&cpld_video_driver);
>> +}
>> +
>>  static struct i2c_board_info __initdata i2c_info[] =  {
>>  {
>>  I2C_BOARD_INFO("24c256", 0x50),
>> @@ -300,6 +347,9 @@ static struct i2c_board_info __initdata i2c_info[] =
> {
>>  {
>>  I2C_BOARD_INFO("cpld_reg0", 0x3a),
>>  },
>> +{
>> +I2C_BOARD_INFO("cpld_video", 0x3B),
>> +},
>>  };
>>  
>>  static struct davinci_i2c_platform_data i2c_pdata = {
>> @@ -307,11 +357,85 @@ static struct davinci_i2c_platform_data i2c_pdata =
> {
>>  .bus_delay  = 0 /* usec */,
>>  };
>>  
>> +static int set_vpif_clock(int mux_mode, int hd)
>> +{
>> +int val = 0;
>> +int err = 0;
>> +unsigned int value;
>> +void __iomem *base = IO_ADDRESS(DAVINCI_SYSTEM_MODULE_BASE);
>> +
>> +if (!cpld_client)
>> +return -ENXIO;
>> +
>> +/* disable the clock */
>> +value = __raw_readl(base + VSCLKDIS_OFFSET);
>> +value |= (VIDCH3CLK | VIDCH2CLK);
>> +__raw_writel(value, base + VSCLKDIS_OFFSET);
>> +
>> +val = i2c_smbus_read_byte(cpld_client);
>> +if (val < 0)
>> +return val;
>> +
>> +if (mux_mode == 1)
>> +val &= ~0x40;
>> +else
>> +val |= 0x40;
>> +
>> +err = i2c_smbus_write_byte(cpld_client, val);
>> +if (err)
>> +return err;
>> +
>> +value = __raw_readl(base + VIDCLKCTL_OFFSET);
>> +value &= ~(VCH2CLK_MASK);
>> +value &= ~(VCH3CLK_MASK);
>> +
>> +if (hd >= 1)
>> +value |= (VCH2CLK_SYSCLK8 | VCH3CLK_SYSCLK8);
>> +else
>> +value |= (VCH2CLK_AUXCLK | VCH3CLK_AUXCLK);
>> +
>> +__raw_writel(value, base + VIDCLKCTL_OFFSET);
>> +
>> +/* enable the clock */
>> +value = __raw_readl(base + VSCLKDIS_OFFSET);
>> +value &= ~(VIDCH3CLK | VIDCH2CLK);
>> +__raw_writel(value, base + VSCLKDIS_OFFSET);
>> +
>> +

Re: [PATCH v4] ARM: DaVinci: DM646x Video: Platform and board specific setup

2009-08-07 Thread Kevin Hilman
Kevin Hilman  writes:

> "chaithrika"  writes:
>
>> Russell,
>>
>> Requesting your ack on this patch.
>>
>
> Chaithrika, Mauro,
>
> We don't need Russell's ack on this.  You've incorporated his comments
> and my signoff is enough for stuff under arch/arm/mach-davinci/*
>
> Mauro, go ahead and merge this.
>

I forgot to add that I'll merge this into DaVinci git while waiting
for it to merge via v4l2.

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


RE: [PATCH v4] ARM: DaVinci: DM646x Video: Platform and board specific setup

2009-08-18 Thread Karicheri, Muralidharan
Russell,

Could you please ack this patch from Chaithrika if you agree with these changes?

I have another set of patches waiting to be submitted and is dependent on this 
patch. So you response will be helpful to speed up the process.

Regards,
Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
email: m-kariche...@ti.com

>-Original Message-
>From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
>ow...@vger.kernel.org] On Behalf Of Subrahmanya, Chaithrika
>Sent: Wednesday, August 05, 2009 1:36 AM
>To: Subrahmanya, Chaithrika; r...@arm.linux.org.uk; li...@arm.linux.org.uk
>Cc: mche...@infradead.org; hverk...@xs4all.nl; linux-media@vger.kernel.org;
>davinci-linux-open-sou...@linux.davincidsp.com; 'Manjunath Hadli'; Jadav,
>Brijesh R; 'Kevin Hilman'
>Subject: RE: [PATCH v4] ARM: DaVinci: DM646x Video: Platform and board
>specific setup
>
>Russell,
>
>Requesting your ack on this patch.
>
>Regards,
>Chaithrika
>
>On Wed, Aug 05, 2009 at 20:17:42, Chaithrika U S wrote:
>> Platform specific display device setup for DM646x EVM
>>
>> Add platform device and resource structures. Also define a platform
>specific
>> clock setup function that can be accessed by the driver to configure the
>clock
>> and CPLD.
>>
>> Signed-off-by: Manjunath Hadli 
>> Signed-off-by: Brijesh Jadav 
>> Signed-off-by: Chaithrika U S 
>> Signed-off-by: Kevin Hilman 
>> ---
>> Applies to Davinci GIT tree. Minor updates like change in structure name-
>> subdev_info to vpif_subdev_info and correction to VDD3P3V_VID_MASK value.
>>
>>  arch/arm/mach-davinci/board-dm646x-evm.c|  125
>+++
>>  arch/arm/mach-davinci/dm646x.c  |   62 +
>>  arch/arm/mach-davinci/include/mach/dm646x.h |   24 +
>>  3 files changed, 211 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c
>b/arch/arm/mach-davinci/board-dm646x-evm.c
>> index b1bf18c..8c88fd0 100644
>> --- a/arch/arm/mach-davinci/board-dm646x-evm.c
>> +++ b/arch/arm/mach-davinci/board-dm646x-evm.c
>> @@ -63,6 +63,19 @@
>>  #define DM646X_EVM_PHY_MASK (0x2)
>>  #define DM646X_EVM_MDIO_FREQUENCY   (220) /* PHY bus frequency */
>>
>> +#define VIDCLKCTL_OFFSET(0x38)
>> +#define VSCLKDIS_OFFSET (0x6c)
>> +
>> +#define VCH2CLK_MASK(BIT_MASK(10) | BIT_MASK(9) | 
>> BIT_MASK(8))
>> +#define VCH2CLK_SYSCLK8 (BIT(9))
>> +#define VCH2CLK_AUXCLK  (BIT(9) | BIT(8))
>> +#define VCH3CLK_MASK(BIT_MASK(14) | BIT_MASK(13) | 
>> BIT_MASK(12))
>> +#define VCH3CLK_SYSCLK8 (BIT(13))
>> +#define VCH3CLK_AUXCLK  (BIT(14) | BIT(13))
>> +
>> +#define VIDCH2CLK   (BIT(10))
>> +#define VIDCH3CLK   (BIT(11))
>> +
>>  static struct davinci_uart_config uart_config __initdata = {
>>  .enabled_uarts = (1 << 0),
>>  };
>> @@ -288,6 +301,40 @@ static struct snd_platform_data
>dm646x_evm_snd_data[]
>= {
>>  },
>>  };
>>
>> +static struct i2c_client *cpld_client;
>> +
>> +static int cpld_video_probe(struct i2c_client *client,
>> +const struct i2c_device_id *id)
>> +{
>> +cpld_client = client;
>> +return 0;
>> +}
>> +
>> +static int __devexit cpld_video_remove(struct i2c_client *client)
>> +{
>> +cpld_client = NULL;
>> +return 0;
>> +}
>> +
>> +static const struct i2c_device_id cpld_video_id[] = {
>> +{ "cpld_video", 0 },
>> +{ }
>> +};
>> +
>> +static struct i2c_driver cpld_video_driver = {
>> +.driver = {
>> +.name   = "cpld_video",
>> +},
>> +.probe  = cpld_video_probe,
>> +.remove = cpld_video_remove,
>> +.id_table   = cpld_video_id,
>> +};
>> +
>> +static void evm_init_cpld(void)
>> +{
>> +i2c_add_driver(&cpld_video_driver);
>> +}
>> +
>>  static struct i2c_board_info __initdata i2c_info[] =  {
>>  {
>>  I2C_BOARD_INFO("24c256", 0x50),
>> @@ -300,6 +347,9 @@ static struct i2c_board_info __initdata i2c_info[] =
>{
>>  {
>>  I2C_BOARD_INFO("cpld_reg0", 0x3a),
>>  },
>> +{
>> +I2C_BOARD_INFO("cpld_video", 0x3B),
>> +},
>>  };
>>
>>  static struct davinci_i2c_platform_data i2c_pdata = {
&g

Re: [PATCH v4] ARM: DaVinci: DM646x Video: Platform and board specific setup

2009-09-01 Thread Russell King - ARM Linux
On Tue, Aug 18, 2009 at 04:58:48PM -0500, Karicheri, Muralidharan wrote:
> Could you please ack this patch from Chaithrika if you agree with these
> changes?

I can only see half the patch here - no idea what the calling convention
is for the set_clock method for example.

> I have another set of patches waiting to be submitted and is dependent
> on this patch. So you response will be helpful to speed up the process.

As published a month before, I've been away.

> >> +static struct i2c_client *cpld_client;
> >> +
> >> +static int cpld_video_probe(struct i2c_client *client,
> >> +  const struct i2c_device_id *id)
> >> +{
> >> +  cpld_client = client;
> >> +  return 0;
> >> +}
> >> +
> >> +static int __devexit cpld_video_remove(struct i2c_client *client)
> >> +{
> >> +  cpld_client = NULL;
> >> +  return 0;
> >> +}

What stops cpld_client being set to NULL while set_vpif_clock() is
trying to use this variable?

I think there should be some locking here, and set_vpif_clock() should
be using the i2c_use_client() / i2c_release_client() interfaces to
ensure safety.

> >> +static int set_vpif_clock(int mux_mode, int hd)
> >> +{
> >> +  int val = 0;
> >> +  int err = 0;
> >> +  unsigned int value;
> >> +  void __iomem *base = IO_ADDRESS(DAVINCI_SYSTEM_MODULE_BASE);
> >> +
> >> +  if (!cpld_client)
> >> +  return -ENXIO;

So here should be:

struct i2c_client *cl;

mutex_lock(&cpld_lock);
cl = i2c_use_client(cpld_client);
mutex_unlock(&cpld_lock);
if (!cl)
return -ENXIO;

and all future references to cpld_client should be made using the local
'cl'.

> >> +
> >> +  /* disable the clock */
> >> +  value = __raw_readl(base + VSCLKDIS_OFFSET);
> >> +  value |= (VIDCH3CLK | VIDCH2CLK);
> >> +  __raw_writel(value, base + VSCLKDIS_OFFSET);
> >> +
> >> +  val = i2c_smbus_read_byte(cpld_client);
> >> +  if (val < 0)
> >> +  return val;
> >> +
> >> +  if (mux_mode == 1)
> >> +  val &= ~0x40;
> >> +  else
> >> +  val |= 0x40;
> >> +
> >> +  err = i2c_smbus_write_byte(cpld_client, val);
> >> +  if (err)
> >> +  return err;
> >> +
> >> +  value = __raw_readl(base + VIDCLKCTL_OFFSET);
> >> +  value &= ~(VCH2CLK_MASK);
> >> +  value &= ~(VCH3CLK_MASK);
> >> +
> >> +  if (hd >= 1)
> >> +  value |= (VCH2CLK_SYSCLK8 | VCH3CLK_SYSCLK8);
> >> +  else
> >> +  value |= (VCH2CLK_AUXCLK | VCH3CLK_AUXCLK);
> >> +
> >> +  __raw_writel(value, base + VIDCLKCTL_OFFSET);
> >> +
> >> +  /* enable the clock */
> >> +  value = __raw_readl(base + VSCLKDIS_OFFSET);
> >> +  value &= ~(VIDCH3CLK | VIDCH2CLK);
> >> +  __raw_writel(value, base + VSCLKDIS_OFFSET);

and:

i2c_release_client(cl);

> >> +
> >> +  return 0;
> >> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html