Re: [RFC] Refactor the cafe_ccic driver and add Armada 610 support
On Tue, 7 Jun 2011, Kassey Lee wrote: hi, Jon: thanks for your work! it is good to know that your brought up MMP2(610) SoC with OV7670 I have some questions: 1) this driver is still based on cafe_ccic.c, as you said, we can abstract the low level register function, and use soc_camera and videofbu2 to manage the buff and state machine, how do you think ? Right, _if_ you really want to usesoc-camera and videobuf2 with your new driver, which you're certainly very welcome to do, all the v4l interfacing has to be removed from the core. Otherwise you can consider re-using it and just implementing a complete v4l2 video bridge / host driver. That's up to you. 2) i2c_adapter, how about move this code to driver/i2c, then ccic driver will become clean? Not sure, what other V4L drivers do? Don't they implement their I2C adapters internally under drivers/video? 3) in mmp_driver.c, it has the sensor name, OV7670, we wish that ccic driver do not need to aware of the sensor, also we need to support front and back camera sensor cases. Yes, that hard-coded bond has to be removed. Thanks Guennadi Guennadi, what is your suggestions ? great thanks! On Tue, Jun 7, 2011 at 6:39 AM, Jonathan Corbet cor...@lwn.net wrote: Hello, all, As I promised last week, here's the state of my work refactoring the Cafe driver and adding Armada 610 support. I intend to have them ready for 3.1, but they are not ready for merging yet. There's a couple of things I'd like to clean up, and I'd like to let the OLPC people test things a bit more. But I figured it would be good to get it out there for comments. Essentially, Marvell has taken the camera controller from the old Cafe chip and dropped it into some ARM SoC setups, one of which is the Armada 610. I pondered just making a new driver, but, given that the controller has changed very little, it made a lot more sense to reuse the existing code. The patches here split cafe_ccic.c into platform and core pieces while leaving functionality unchanged. The new mmp-camera driver is then added as a second platform. This work is not done; at a minimum, I plan to convert it over to videobuf2 and make use of the Armada's s/g DMA capabilities. Doubtless there is plenty more to be done; I would also sure like to see Kassey Lee's Marvell driver integrated with this one if at all possible. Comments? Thanks, jon -- 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 -- Best regards Kassey Application Processor Systems Engineering, Marvell Technology Group Ltd. Shanghai, China. --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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: [RFC] Refactor the cafe_ccic driver and add Armada 610 support
hi, Jon: thanks for your work! it is good to know that your brought up MMP2(610) SoC with OV7670 I have some questions: 1) this driver is still based on cafe_ccic.c, as you said, we can abstract the low level register function, and use soc_camera and videofbu2 to manage the buff and state machine, how do you think ? 2) i2c_adapter, how about move this code to driver/i2c, then ccic driver will become clean? 3) in mmp_driver.c, it has the sensor name, OV7670, we wish that ccic driver do not need to aware of the sensor, also we need to support front and back camera sensor cases. Guennadi, what is your suggestions ? great thanks! On Tue, Jun 7, 2011 at 6:39 AM, Jonathan Corbet cor...@lwn.net wrote: Hello, all, As I promised last week, here's the state of my work refactoring the Cafe driver and adding Armada 610 support. I intend to have them ready for 3.1, but they are not ready for merging yet. There's a couple of things I'd like to clean up, and I'd like to let the OLPC people test things a bit more. But I figured it would be good to get it out there for comments. Essentially, Marvell has taken the camera controller from the old Cafe chip and dropped it into some ARM SoC setups, one of which is the Armada 610. I pondered just making a new driver, but, given that the controller has changed very little, it made a lot more sense to reuse the existing code. The patches here split cafe_ccic.c into platform and core pieces while leaving functionality unchanged. The new mmp-camera driver is then added as a second platform. This work is not done; at a minimum, I plan to convert it over to videobuf2 and make use of the Armada's s/g DMA capabilities. Doubtless there is plenty more to be done; I would also sure like to see Kassey Lee's Marvell driver integrated with this one if at all possible. Comments? Thanks, jon -- 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 -- Best regards Kassey Application Processor Systems Engineering, Marvell Technology Group Ltd. Shanghai, China. -- 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: [RFC] Refactor the cafe_ccic driver and add Armada 610 support
On Tue, 7 Jun 2011 13:30:11 +0800 Kassey Lee kassey1...@gmail.com wrote: 1) this driver is still based on cafe_ccic.c, as you said, we can abstract the low level register function, and use soc_camera and videofbu2 to manage the buff and state machine, how do you think ? As I said, videobuf2 is on my list of things to do. Note that the driver works just fine without - that code has been in the kernel and working for years. But it's a cleanup that needs to be done at this point, and I will do it. 2) i2c_adapter, how about move this code to driver/i2c, then ccic driver will become clean? The problem there is that you can't easily disentangle the two devices - they use the same registers, the same IRQ line, etc. One *could* turn the whole thing into an MFD driver and split them apart, but I honestly don't see a reason to do that. I'd be surprised if a Cafe chip ever shows up in anything new these days, so it's only used in the OLPC XO 1, and that i2c will never be used for anything but the sensor. The i2c *has* been abstracted out of the camera core, so the Cafe i2c implementation will not get in the way of any new drivers. 3) in mmp_driver.c, it has the sensor name, OV7670, we wish that ccic driver do not need to aware of the sensor, also we need to support front and back camera sensor cases. The only reason the ov7670 dependency is there is because that's the only sensor the driver has ever been used with. Adding other sensors has been complicated a bit by Hans's changes which pushed awareness of the available video formats into the controller driver (I complained at the time), but that can be worked around. For front and back: I didn't wire up the second controller in the mmp driver because I don't have a device that uses both. I very much wrote the driver with the idea that both controllers could be used, though; finishing that job will be easy. One thing I haven't done is to look at your driver closely enough to think about whether mmp_camera can drive your hardware or not. You'll have a better idea of that than me, I suspect; is the hardware pretty much the same? Thanks, jon -- 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: [RFC] Refactor the cafe_ccic driver and add Armada 610 support
Hi, Jon: for PXA910, MMP2(Armda 610) actually, CCIC are almost the same. but MMP2 has 2 CCIC controllers, and PXA910 only has 1 CCIC controller. so it is quite make your driver works on MMP2 and PXA910. On Tue, Jun 7, 2011 at 9:45 PM, Jonathan Corbet cor...@lwn.net wrote: On Tue, 7 Jun 2011 13:30:11 +0800 Kassey Lee kassey1...@gmail.com wrote: 1) this driver is still based on cafe_ccic.c, as you said, we can abstract the low level register function, and use soc_camera and videofbu2 to manage the buff and state machine, how do you think ? As I said, videobuf2 is on my list of things to do. Note that the driver works just fine without - that code has been in the kernel and working for years. But it's a cleanup that needs to be done at this point, and I will do it. Since we have converted a videfbuf2 version for PXA910, could you have time to review ? and find the common part to make a better one works on cafe-ccic, PXA910, and MMP2 ? it takes quite time to convert from cafe-ccic.c to videfbuf2 for me. 2) i2c_adapter, how about move this code to driver/i2c, then ccic driver will become clean? The problem there is that you can't easily disentangle the two devices - they use the same registers, the same IRQ line, etc. One *could* turn the whole thing into an MFD driver and split them apart, but I honestly don't see a reason to do that. I'd be surprised if a Cafe chip ever shows up in anything new these days, so it's only used in the OLPC XO 1, and that i2c will never be used for anything but the sensor. The i2c *has* been abstracted out of the camera core, so the Cafe i2c implementation will not get in the way of any new drivers. for cafe i2c, it is OK. but for PXA910 and MMP2, the i2c adapter is out of CCIC. 3) in mmp_driver.c, it has the sensor name, OV7670, we wish that ccic driver do not need to aware of the sensor, also we need to support front and back camera sensor cases. The only reason the ov7670 dependency is there is because that's the only sensor the driver has ever been used with. Adding other sensors has been complicated a bit by Hans's changes which pushed awareness of the available video formats into the controller driver (I complained at the time), but that can be worked around. For front and back: I didn't wire up the second controller in the mmp driver because I don't have a device that uses both. I very much wrote the driver with the idea that both controllers could be used, though; finishing that job will be easy. sorry for confusion, we have another case: one CCIC should work with two sensors, and switch them by need. what i mean, for CCIC driver it should not aware the sensor, it can be done by user application. One thing I haven't done is to look at your driver closely enough to think about whether mmp_camera can drive your hardware or not. You'll have a better idea of that than me, I suspect; is the hardware pretty much the same? I will update my driver again after Guennadi's comments, would you please have time to review ? 1) add Jon's email, and cafe-ccic.c copyright. 2) set_bus_param check for HSYNC, PCLK 3) code stye fix. Thanks, jon -- Best regards Kassey Application Processor Systems Engineering, Marvell Technology Group Ltd. Shanghai, China. -- 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
