linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL FOR v3.10] Camera sensors patches
@ 2013-04-12  9:13 Laurent Pinchart
  2013-04-14 19:59 ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 14+ messages in thread
From: Laurent Pinchart @ 2013-04-12  9:13 UTC (permalink / raw)
  To: linux-media

Hi Mauro,

The following changes since commit 81e096c8ac6a064854c2157e0bf802dc4906678c:

  [media] budget: Add support for Philips Semi Sylt PCI ref. design 
(2013-04-08 07:28:01 -0300)

are available in the git repository at:

  git://linuxtv.org/pinchartl/media.git sensors/next

for you to fetch changes up to c890926a06339944790c5c265e21e8547aa55e49:

  mt9p031: Use the common clock framework (2013-04-12 11:07:07 +0200)

----------------------------------------------------------------
Laurent Pinchart (5):
      mt9m032: Fix PLL setup
      mt9m032: Define MT9M032_READ_MODE1 bits
      mt9p031: Use devm_* managed helpers
      mt9p031: Add support for regulators
      mt9p031: Use the common clock framework

 drivers/media/i2c/mt9m032.c | 46 +++++++++++++++++++++++++++++++-----------
 drivers/media/i2c/mt9p031.c | 58 ++++++++++++++++++++++++++++++--------------
 include/media/mt9p031.h     |  2 --
 3 files changed, 75 insertions(+), 31 deletions(-)

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-12  9:13 [GIT PULL FOR v3.10] Camera sensors patches Laurent Pinchart
@ 2013-04-14 19:59 ` Mauro Carvalho Chehab
  2013-04-15 10:19   ` Laurent Pinchart
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2013-04-14 19:59 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media

Em Fri, 12 Apr 2013 11:13:06 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Mauro,
> 
> The following changes since commit 81e096c8ac6a064854c2157e0bf802dc4906678c:
> 
>   [media] budget: Add support for Philips Semi Sylt PCI ref. design 
> (2013-04-08 07:28:01 -0300)
> 
> are available in the git repository at:
> 
>   git://linuxtv.org/pinchartl/media.git sensors/next
> 
> for you to fetch changes up to c890926a06339944790c5c265e21e8547aa55e49:
> 
>   mt9p031: Use the common clock framework (2013-04-12 11:07:07 +0200)
> 
> ----------------------------------------------------------------
> Laurent Pinchart (5):
>       mt9m032: Fix PLL setup
>       mt9m032: Define MT9M032_READ_MODE1 bits
>       mt9p031: Use devm_* managed helpers

>       mt9p031: Add support for regulators
>       mt9p031: Use the common clock framework

Hmm... It seems ugly to have regulators and clock framework and other
SoC calls inside an i2c driver that can be used by a device that doesn't
have regulators.

I'm not sure what's the best solution for it, so, I'll be adding those
two patches, but it seems that we'll need to restrict the usage of those
calls only if the caller driver is a platform driver.

Regards,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-14 19:59 ` Mauro Carvalho Chehab
@ 2013-04-15 10:19   ` Laurent Pinchart
  2013-04-15 12:42     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 14+ messages in thread
From: Laurent Pinchart @ 2013-04-15 10:19 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media

Hi Mauro,

On Sunday 14 April 2013 16:59:58 Mauro Carvalho Chehab wrote:
> Em Fri, 12 Apr 2013 11:13:06 +0200 Laurent Pinchart escreveu:
> > Hi Mauro,
> > 
> > The following changes since commit 
81e096c8ac6a064854c2157e0bf802dc4906678c:
> >   [media] budget: Add support for Philips Semi Sylt PCI ref. design
> > 
> > (2013-04-08 07:28:01 -0300)
> > 
> > are available in the git repository at:
> >   git://linuxtv.org/pinchartl/media.git sensors/next
> > 
> > for you to fetch changes up to c890926a06339944790c5c265e21e8547aa55e49:
> >   mt9p031: Use the common clock framework (2013-04-12 11:07:07 +0200)
> > 
> > ----------------------------------------------------------------
> > 
> > Laurent Pinchart (5):
> >       mt9m032: Fix PLL setup
> >       mt9m032: Define MT9M032_READ_MODE1 bits
> >       mt9p031: Use devm_* managed helpers
> >       mt9p031: Add support for regulators
> >       mt9p031: Use the common clock framework
> 
> Hmm... It seems ugly to have regulators and clock framework and other SoC
> calls inside an i2c driver that can be used by a device that doesn't have
> regulators.
> 
> I'm not sure what's the best solution for it, so, I'll be adding those two
> patches, but it seems that we'll need to restrict the usage of those calls
> only if the caller driver is a platform driver.

The MT9P031 needs power supplies and a clock on all platforms, regardless of 
the bridge bus type. I suppose the use case that mostly concerns you here is 
USB webcams where the power supplies and the clock are controlled 
automatically by the device. If we ever need to support such a device in the 
future we can of course revisit the driver then, and one possible solution 
would be to register fixed voltage regulators and a fixed clock.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-15 10:19   ` Laurent Pinchart
@ 2013-04-15 12:42     ` Mauro Carvalho Chehab
  2013-04-16 15:30       ` Laurent Pinchart
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2013-04-15 12:42 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media

Em Mon, 15 Apr 2013 12:19:23 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Mauro,
> 
> On Sunday 14 April 2013 16:59:58 Mauro Carvalho Chehab wrote:
> > Em Fri, 12 Apr 2013 11:13:06 +0200 Laurent Pinchart escreveu:
> > > Hi Mauro,
> > > 
> > > The following changes since commit 
> 81e096c8ac6a064854c2157e0bf802dc4906678c:
> > >   [media] budget: Add support for Philips Semi Sylt PCI ref. design
> > > 
> > > (2013-04-08 07:28:01 -0300)
> > > 
> > > are available in the git repository at:
> > >   git://linuxtv.org/pinchartl/media.git sensors/next
> > > 
> > > for you to fetch changes up to c890926a06339944790c5c265e21e8547aa55e49:
> > >   mt9p031: Use the common clock framework (2013-04-12 11:07:07 +0200)
> > > 
> > > ----------------------------------------------------------------
> > > 
> > > Laurent Pinchart (5):
> > >       mt9m032: Fix PLL setup
> > >       mt9m032: Define MT9M032_READ_MODE1 bits
> > >       mt9p031: Use devm_* managed helpers
> > >       mt9p031: Add support for regulators
> > >       mt9p031: Use the common clock framework
> > 
> > Hmm... It seems ugly to have regulators and clock framework and other SoC
> > calls inside an i2c driver that can be used by a device that doesn't have
> > regulators.
> > 
> > I'm not sure what's the best solution for it, so, I'll be adding those two
> > patches, but it seems that we'll need to restrict the usage of those calls
> > only if the caller driver is a platform driver.
> 
> The MT9P031 needs power supplies and a clock on all platforms, regardless of 
> the bridge bus type. 

Well, all digital devices require clock and power. If power is either a
simple electric circuit, a battery or a regulator, that depends on the board.

> I suppose the use case that mostly concerns you here is 
> USB webcams 

Yes.

> where the power supplies and the clock are controlled 
> automatically by the device. 

Or could be not controlled at all. It could be a simple XTAL attached to the
sensor or a clock signal provided by the bridge obtained from a fixed XTAL,
and a resistor bridge or a Zenner diode providing the needed power voltage.

> If we ever need to support such a device in the 
> future we can of course revisit the driver then, and one possible solution 
> would be to register fixed voltage regulators and a fixed clock.

That is an overkill: devices were the power supply/xtal clock can't be
controlled should not require extra software that pretend to control it.

Regards,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-15 12:42     ` Mauro Carvalho Chehab
@ 2013-04-16 15:30       ` Laurent Pinchart
  2013-04-16 17:36         ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 14+ messages in thread
From: Laurent Pinchart @ 2013-04-16 15:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media

Hi Mauro,

On Monday 15 April 2013 09:42:48 Mauro Carvalho Chehab wrote:
> Em Mon, 15 Apr 2013 12:19:23 +0200 Laurent Pinchart escreveu:
> > On Sunday 14 April 2013 16:59:58 Mauro Carvalho Chehab wrote:
> > > Em Fri, 12 Apr 2013 11:13:06 +0200 Laurent Pinchart escreveu:
> > > > Hi Mauro,
> > > > 
> > > > The following changes since commit
> > 
> > 81e096c8ac6a064854c2157e0bf802dc4906678c:
> > > >   [media] budget: Add support for Philips Semi Sylt PCI ref. design
> > > > 
> > > > (2013-04-08 07:28:01 -0300)
> > > > 
> > > > are available in the git repository at:
> > > >   git://linuxtv.org/pinchartl/media.git sensors/next
> > > > 
> > > > for you to fetch changes up to 
c890926a06339944790c5c265e21e8547aa55e49:
> > > >   mt9p031: Use the common clock framework (2013-04-12 11:07:07 +0200)
> > > > 
> > > > ----------------------------------------------------------------
> > > > 
> > > > Laurent Pinchart (5):
> > > >       mt9m032: Fix PLL setup
> > > >       mt9m032: Define MT9M032_READ_MODE1 bits
> > > >       mt9p031: Use devm_* managed helpers
> > > >       mt9p031: Add support for regulators
> > > >       mt9p031: Use the common clock framework
> > > 
> > > Hmm... It seems ugly to have regulators and clock framework and other
> > > SoC calls inside an i2c driver that can be used by a device that doesn't
> > > have regulators.
> > > 
> > > I'm not sure what's the best solution for it, so, I'll be adding those
> > > two patches, but it seems that we'll need to restrict the usage of those
> > > calls only if the caller driver is a platform driver.
> > 
> > The MT9P031 needs power supplies and a clock on all platforms, regardless
> > of the bridge bus type.
> 
> Well, all digital devices require clock and power. If power is either a
> simple electric circuit, a battery or a regulator, that depends on the
> board.
>
> > I suppose the use case that mostly concerns you here is
> > USB webcams
> 
> Yes.
> 
> > where the power supplies and the clock are controlled automatically by the
> > device.
> 
> Or could be not controlled at all. It could be a simple XTAL attached to the
> sensor or a clock signal provided by the bridge obtained from a fixed XTAL,
> and a resistor bridge or a Zenner diode providing the needed power voltage.
>
> > If we ever need to support such a device in the future we can of course
> > revisit the driver then, and one possible solution would be to register
> > fixed voltage regulators and a fixed clock.
> 
> That is an overkill: devices were the power supply/xtal clock can't be
> controlled should not require extra software that pretend to control it.

If I'm not mistaken that's however the recommended way on embedded devices at 
the moment. I don't have a strong opinion on the subject for now, but this 
will need to be at least discussed with core clock and regulator developers.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-16 15:30       ` Laurent Pinchart
@ 2013-04-16 17:36         ` Mauro Carvalho Chehab
  2013-04-16 18:04           ` Sylwester Nawrocki
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2013-04-16 17:36 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media

Em 16-04-2013 12:30, Laurent Pinchart escreveu:
> Hi Mauro,
>
> On Monday 15 April 2013 09:42:48 Mauro Carvalho Chehab wrote:
>> Em Mon, 15 Apr 2013 12:19:23 +0200 Laurent Pinchart escreveu:
>>> On Sunday 14 April 2013 16:59:58 Mauro Carvalho Chehab wrote:
>>>> Em Fri, 12 Apr 2013 11:13:06 +0200 Laurent Pinchart escreveu:
>>>>> Hi Mauro,
>>>>>
>>>>> The following changes since commit
>>>
>>> 81e096c8ac6a064854c2157e0bf802dc4906678c:
>>>>>    [media] budget: Add support for Philips Semi Sylt PCI ref. design
>>>>>
>>>>> (2013-04-08 07:28:01 -0300)
>>>>>
>>>>> are available in the git repository at:
>>>>>    git://linuxtv.org/pinchartl/media.git sensors/next
>>>>>
>>>>> for you to fetch changes up to
> c890926a06339944790c5c265e21e8547aa55e49:
>>>>>    mt9p031: Use the common clock framework (2013-04-12 11:07:07 +0200)
>>>>>
>>>>> ----------------------------------------------------------------
>>>>>
>>>>> Laurent Pinchart (5):
>>>>>        mt9m032: Fix PLL setup
>>>>>        mt9m032: Define MT9M032_READ_MODE1 bits
>>>>>        mt9p031: Use devm_* managed helpers
>>>>>        mt9p031: Add support for regulators
>>>>>        mt9p031: Use the common clock framework
>>>>
>>>> Hmm... It seems ugly to have regulators and clock framework and other
>>>> SoC calls inside an i2c driver that can be used by a device that doesn't
>>>> have regulators.
>>>>
>>>> I'm not sure what's the best solution for it, so, I'll be adding those
>>>> two patches, but it seems that we'll need to restrict the usage of those
>>>> calls only if the caller driver is a platform driver.
>>>
>>> The MT9P031 needs power supplies and a clock on all platforms, regardless
>>> of the bridge bus type.
>>
>> Well, all digital devices require clock and power. If power is either a
>> simple electric circuit, a battery or a regulator, that depends on the
>> board.
>>
>>> I suppose the use case that mostly concerns you here is
>>> USB webcams
>>
>> Yes.
>>
>>> where the power supplies and the clock are controlled automatically by the
>>> device.
>>
>> Or could be not controlled at all. It could be a simple XTAL attached to the
>> sensor or a clock signal provided by the bridge obtained from a fixed XTAL,
>> and a resistor bridge or a Zenner diode providing the needed power voltage.
>>
>>> If we ever need to support such a device in the future we can of course
>>> revisit the driver then, and one possible solution would be to register
>>> fixed voltage regulators and a fixed clock.
>>
>> That is an overkill: devices were the power supply/xtal clock can't be
>> controlled should not require extra software that pretend to control it.
>
> If I'm not mistaken that's however the recommended way on embedded devices at
> the moment. I don't have a strong opinion on the subject for now, but this
> will need to be at least discussed with core clock and regulator developers.


Well, a customer's webcam is not an embedded device at all. That's why
I think that putting it at the I2C driver is wrong: those drivers are
not to be used only by embedded hardware.

Regards,
Mauro

>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-16 17:36         ` Mauro Carvalho Chehab
@ 2013-04-16 18:04           ` Sylwester Nawrocki
  2013-04-17 13:55             ` Mark Brown
  0 siblings, 1 reply; 14+ messages in thread
From: Sylwester Nawrocki @ 2013-04-16 18:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Laurent Pinchart, linux-media, Mark Brown, Mike Turquette

Cc: Mark, Mike

On 04/16/2013 07:36 PM, Mauro Carvalho Chehab wrote:
> Em 16-04-2013 12:30, Laurent Pinchart escreveu:
>> Hi Mauro,
>>
>> On Monday 15 April 2013 09:42:48 Mauro Carvalho Chehab wrote:
>>> Em Mon, 15 Apr 2013 12:19:23 +0200 Laurent Pinchart escreveu:
>>>> On Sunday 14 April 2013 16:59:58 Mauro Carvalho Chehab wrote:
>>>>> Em Fri, 12 Apr 2013 11:13:06 +0200 Laurent Pinchart escreveu:
>>>>>> Hi Mauro,
>>>>>>
>>>>>> The following changes since commit
>>>>
>>>> 81e096c8ac6a064854c2157e0bf802dc4906678c:
>>>>>>    [media] budget: Add support for Philips Semi Sylt PCI ref. design
>>>>>>
>>>>>> (2013-04-08 07:28:01 -0300)
>>>>>>
>>>>>> are available in the git repository at:
>>>>>>    git://linuxtv.org/pinchartl/media.git sensors/next
>>>>>>
>>>>>> for you to fetch changes up to
>> c890926a06339944790c5c265e21e8547aa55e49:
>>>>>>    mt9p031: Use the common clock framework (2013-04-12 11:07:07 +0200)
>>>>>>
>>>>>> ----------------------------------------------------------------
>>>>>>
>>>>>> Laurent Pinchart (5):
>>>>>>        mt9m032: Fix PLL setup
>>>>>>        mt9m032: Define MT9M032_READ_MODE1 bits
>>>>>>        mt9p031: Use devm_* managed helpers
>>>>>>        mt9p031: Add support for regulators
>>>>>>        mt9p031: Use the common clock framework
>>>>>
>>>>> Hmm... It seems ugly to have regulators and clock framework and other
>>>>> SoC calls inside an i2c driver that can be used by a device that doesn't
>>>>> have regulators.
>>>>>
>>>>> I'm not sure what's the best solution for it, so, I'll be adding those
>>>>> two patches, but it seems that we'll need to restrict the usage of those
>>>>> calls only if the caller driver is a platform driver.
>>>>
>>>> The MT9P031 needs power supplies and a clock on all platforms, regardless
>>>> of the bridge bus type.
>>>
>>> Well, all digital devices require clock and power. If power is either a
>>> simple electric circuit, a battery or a regulator, that depends on the
>>> board.
>>>
>>>> I suppose the use case that mostly concerns you here is
>>>> USB webcams
>>>
>>> Yes.
>>>
>>>> where the power supplies and the clock are controlled automatically by the
>>>> device.
>>>
>>> Or could be not controlled at all. It could be a simple XTAL attached to the
>>> sensor or a clock signal provided by the bridge obtained from a fixed XTAL,
>>> and a resistor bridge or a Zenner diode providing the needed power voltage.

Yes, and this are details of the whole system, which IMHO are supposed to be
abstracted outside of a clock/power supply consumer driver. What resources
are needed is well defined by a chip architecture, driver of a chip knows what
voltage supply/clocks, etc. it needs and it should request that. Now on some
systems there is no need to explicitly enable/disable, change parameters of
some clocks/voltage regulators. And the question is, IIUC, at what layer we
choose to abstract such differences. I don't think it is a good idea to push
it into a sub-device driver. A sub-device driver would need to know details
of its host driver, wouldn't it ? AFAICT it's not something subdev drivers are
really supposed to deal with.

I'm not sure if the regulators and the clock framework are "SoC calls". These
are generic APIs that have well defined semantic of abstracting platform
differences. At least it can be said about the regulators API IMHO.

It's probably more clean to provide a dummy clock/regulator in a host driver
(platform) than to add something in a sub-device drivers that would resolve
which resources should be requested and which not.

>>>> If we ever need to support such a device in the future we can of course
>>>> revisit the driver then, and one possible solution would be to register
>>>> fixed voltage regulators and a fixed clock.
>>>
>>> That is an overkill: devices were the power supply/xtal clock can't be
>>> controlled should not require extra software that pretend to control it.
>>
>> If I'm not mistaken that's however the recommended way on embedded devices at
>> the moment. I don't have a strong opinion on the subject for now, but this
>> will need to be at least discussed with core clock and regulator developers.
> 
> 
> Well, a customer's webcam is not an embedded device at all. That's why
> I think that putting it at the I2C driver is wrong: those drivers are
> not to be used only by embedded hardware.

Regards,
Sylwester

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-16 18:04           ` Sylwester Nawrocki
@ 2013-04-17 13:55             ` Mark Brown
  2013-04-17 14:36               ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 14+ messages in thread
From: Mark Brown @ 2013-04-17 13:55 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Mauro Carvalho Chehab, Laurent Pinchart, linux-media,
	Mike Turquette

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

On Tue, Apr 16, 2013 at 08:04:52PM +0200, Sylwester Nawrocki wrote:

> It's probably more clean to provide a dummy clock/regulator in a host driver
> (platform) than to add something in a sub-device drivers that would resolve
> which resources should be requested and which not.

Yes, that's the general theory for regulators at least - it allows the
device driver to just trundle along and not worry about how the board is
hooked up.  The other issue it resolves that you didn't mention is that
it avoids just ignoring errors which isn't terribly clever.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-17 13:55             ` Mark Brown
@ 2013-04-17 14:36               ` Mauro Carvalho Chehab
  2013-04-21 23:14                 ` Laurent Pinchart
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2013-04-17 14:36 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mark Brown, Sylwester Nawrocki, linux-media, Mike Turquette

Em Wed, 17 Apr 2013 14:55:03 +0100
Mark Brown <broonie@kernel.org> escreveu:

> On Tue, Apr 16, 2013 at 08:04:52PM +0200, Sylwester Nawrocki wrote:
> 
> > It's probably more clean to provide a dummy clock/regulator in a host driver
> > (platform) than to add something in a sub-device drivers that would resolve
> > which resources should be requested and which not.
> 
> Yes, that's the general theory for regulators at least - it allows the
> device driver to just trundle along and not worry about how the board is
> hooked up.  The other issue it resolves that you didn't mention is that
> it avoids just ignoring errors which isn't terribly clever.

I agree. Adding dummy clock/regulator at the host platform driver makes
sense, as the platform driver knows how the board is hooked up; keeping 
it at the I2C driver doesn't make sense, so the code needs to be moved
away from it.

Laurent,

Could you please work on a patch moving that code to the host platform
driver?

Thanks!
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-17 14:36               ` Mauro Carvalho Chehab
@ 2013-04-21 23:14                 ` Laurent Pinchart
  2013-04-22 10:03                   ` Mark Brown
  0 siblings, 1 reply; 14+ messages in thread
From: Laurent Pinchart @ 2013-04-21 23:14 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Mark Brown, Sylwester Nawrocki, linux-media, Mike Turquette

Hi Mauro,

On Wednesday 17 April 2013 11:36:39 Mauro Carvalho Chehab wrote:
> Em Wed, 17 Apr 2013 14:55:03 +0100 Mark Brown escreveu:
> > On Tue, Apr 16, 2013 at 08:04:52PM +0200, Sylwester Nawrocki wrote:
> > > It's probably more clean to provide a dummy clock/regulator in a host
> > > driver (platform) than to add something in a sub-device drivers that
> > > would resolve which resources should be requested and which not.
> > 
> > Yes, that's the general theory for regulators at least - it allows the
> > device driver to just trundle along and not worry about how the board is
> > hooked up.  The other issue it resolves that you didn't mention is that
> > it avoids just ignoring errors which isn't terribly clever.
> 
> I agree. Adding dummy clock/regulator at the host platform driver makes
> sense, as the platform driver knows how the board is hooked up; keeping
> it at the I2C driver doesn't make sense, so the code needs to be moved
> away from it.
> 
> Laurent,
> 
> Could you please work on a patch moving that code to the host platform
> driver?

I think that Mark's point was that the regulators should be provided by 
platform code (in the generic sense, it could be DT on ARM, board code, or a 
USB bridge driver for a webcam that uses the mt9p031 sensor) and used by the 
sensor driver. That's exactly what my mt9p031 patch does.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-21 23:14                 ` Laurent Pinchart
@ 2013-04-22 10:03                   ` Mark Brown
  2013-04-22 12:46                     ` Mauro Carvalho Chehab
  2013-04-22 12:46                     ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 14+ messages in thread
From: Mark Brown @ 2013-04-22 10:03 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, Sylwester Nawrocki, linux-media,
	Mike Turquette

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

On Mon, Apr 22, 2013 at 01:14:07AM +0200, Laurent Pinchart wrote:

> I think that Mark's point was that the regulators should be provided by 
> platform code (in the generic sense, it could be DT on ARM, board code, or a 
> USB bridge driver for a webcam that uses the mt9p031 sensor) and used by the 
> sensor driver. That's exactly what my mt9p031 patch does.

Yes, you understood me perfectly - to a good approximation the matching
up should be done by whatever the chip is soldered down to.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-22 10:03                   ` Mark Brown
@ 2013-04-22 12:46                     ` Mauro Carvalho Chehab
  2013-04-22 12:56                       ` Mark Brown
  2013-04-22 12:46                     ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2013-04-22 12:46 UTC (permalink / raw)
  To: Mark Brown
  Cc: Laurent Pinchart, Sylwester Nawrocki, linux-media, Mike Turquette

Em 22-04-2013 07:03, Mark Brown escreveu:
> On Mon, Apr 22, 2013 at 01:14:07AM +0200, Laurent Pinchart wrote:
>
>> I think that Mark's point was that the regulators should be provided by
>> platform code (in the generic sense, it could be DT on ARM, board code, or a
>> USB bridge driver for a webcam that uses the mt9p031 sensor) and used by the
>> sensor driver. That's exactly what my mt9p031 patch does.
>
> Yes, you understood me perfectly - to a good approximation the matching
> up should be done by whatever the chip is soldered down to.
>

That doesn't make any sense to me. I2C devices can be used anywere,
as they can be soldered either internally on an USB webcam without
any regulators or any other platform code on it or could be soldered
to some platform-specific bus.

Also, what best describes "soldered" here is the binding between
an I2C driver and the I2C adapter. The I2C adapter is a platform
driver on embedded devices, where, on an usual USB camera, it
is just a USB->I2C bridge.

Also, requiring that simple USB cameras to have regulators will
prevent its usual usage, as non-platform distros don't set config
REGULATOR, and it shouldn't.

Regards,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-22 10:03                   ` Mark Brown
  2013-04-22 12:46                     ` Mauro Carvalho Chehab
@ 2013-04-22 12:46                     ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2013-04-22 12:46 UTC (permalink / raw)
  To: Mark Brown
  Cc: Laurent Pinchart, Sylwester Nawrocki, linux-media, Mike Turquette

Em 22-04-2013 07:03, Mark Brown escreveu:
> On Mon, Apr 22, 2013 at 01:14:07AM +0200, Laurent Pinchart wrote:
>
>> I think that Mark's point was that the regulators should be provided by
>> platform code (in the generic sense, it could be DT on ARM, board code, or a
>> USB bridge driver for a webcam that uses the mt9p031 sensor) and used by the
>> sensor driver. That's exactly what my mt9p031 patch does.
>
> Yes, you understood me perfectly - to a good approximation the matching
> up should be done by whatever the chip is soldered down to.
>

That doesn't make any sense to me. I2C devices can be used anywere,
as they can be soldered either internally on an USB webcam without
any regulators or any other platform code on it or could be soldered
to some platform-specific bus.

Also, what best describes "soldered" here is the binding between
an I2C driver and the I2C adapter. The I2C adapter is a platform
driver on embedded devices, where, on an usual USB camera, it
is just a USB->I2C bridge.

Also, requiring that simple USB cameras to have regulators will
prevent its usual usage, as non-platform distros don't set config
REGULATOR (and they shouldn't, as that would just increase the
Kernel's footprint for a code that will never ever be needed there).

Regards,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL FOR v3.10] Camera sensors patches
  2013-04-22 12:46                     ` Mauro Carvalho Chehab
@ 2013-04-22 12:56                       ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2013-04-22 12:56 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Laurent Pinchart, Sylwester Nawrocki, linux-media, Mike Turquette

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

On Mon, Apr 22, 2013 at 09:46:07AM -0300, Mauro Carvalho Chehab wrote:
> Em 22-04-2013 07:03, Mark Brown escreveu:

> >Yes, you understood me perfectly - to a good approximation the matching
> >up should be done by whatever the chip is soldered down to.

> That doesn't make any sense to me. I2C devices can be used anywere,
> as they can be soldered either internally on an USB webcam without
> any regulators or any other platform code on it or could be soldered
> to some platform-specific bus.

If it's running on Linux on a visible I2C bus it ought to be shown as an
I2C bus on Linux and the thing doing that plumbing ought to be worrying
about hooking up anything the driver needs.

> Also, what best describes "soldered" here is the binding between
> an I2C driver and the I2C adapter. The I2C adapter is a platform
> driver on embedded devices, where, on an usual USB camera, it
> is just a USB->I2C bridge.

Sure, but there's no meaningful difference between these things as far
as plumbing things together goes.

> Also, requiring that simple USB cameras to have regulators will
> prevent its usual usage, as non-platform distros don't set config
> REGULATOR, and it shouldn't.

No problem there, the regulator API stubs itself out if it's not
enabled.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-04-22 12:56 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-12  9:13 [GIT PULL FOR v3.10] Camera sensors patches Laurent Pinchart
2013-04-14 19:59 ` Mauro Carvalho Chehab
2013-04-15 10:19   ` Laurent Pinchart
2013-04-15 12:42     ` Mauro Carvalho Chehab
2013-04-16 15:30       ` Laurent Pinchart
2013-04-16 17:36         ` Mauro Carvalho Chehab
2013-04-16 18:04           ` Sylwester Nawrocki
2013-04-17 13:55             ` Mark Brown
2013-04-17 14:36               ` Mauro Carvalho Chehab
2013-04-21 23:14                 ` Laurent Pinchart
2013-04-22 10:03                   ` Mark Brown
2013-04-22 12:46                     ` Mauro Carvalho Chehab
2013-04-22 12:56                       ` Mark Brown
2013-04-22 12:46                     ` Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).