Re: [GIT PULL FOR v3.10] Camera sensors patches
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. signature.asc Description: Digital signature
Re: [GIT PULL FOR v3.10] Camera sensors patches
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 -- 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: [GIT PULL FOR v3.10] Camera sensors patches
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 -- 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: [GIT PULL FOR v3.10] Camera sensors patches
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. signature.asc Description: Digital signature
Re: [GIT PULL FOR v3.10] Camera sensors patches
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 -- 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: [GIT PULL FOR v3.10] Camera sensors patches
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. signature.asc Description: Digital signature
Re: [GIT PULL FOR v3.10] Camera sensors patches
Em Wed, 17 Apr 2013 14:55:03 +0100 Mark Brown broo...@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 -- 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: [GIT PULL FOR v3.10] Camera sensors patches
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 -- 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: [GIT PULL FOR v3.10] Camera sensors patches
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 -- 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: [GIT PULL FOR v3.10] Camera sensors patches
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 -- 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: [GIT PULL FOR v3.10] Camera sensors patches
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 -- 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: [GIT PULL FOR v3.10] Camera sensors patches
Em Mon, 15 Apr 2013 12:19:23 +0200 Laurent Pinchart laurent.pinch...@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 -- 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: [GIT PULL FOR v3.10] Camera sensors patches
Em Fri, 12 Apr 2013 11:13:06 +0200 Laurent Pinchart laurent.pinch...@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 -- 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
