| Message ID | 20190528090731.10341-6-lkundrak@v3.sk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: make Marvell camera work on DT-based OLPC XO-1.75 | expand |
On Tue, May 28, 2019 at 11:07:29AM +0200, Lubomir Rintel wrote: > The platform data is actually not used anywhere (along with the CSI > support) and should be safe to remove. > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> > Acked-by: Pavel Machek <pavel@ucw.cz> > > --- > Changes since v1: > - s/This are/These are/ in a comment > > .../media/platform/marvell-ccic/mmp-driver.c | 36 ++++++++++++++----- > 1 file changed, 27 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c > index 8f1d2a972bc9..54c2dd8c29d8 100644 > --- a/drivers/media/platform/marvell-ccic/mmp-driver.c > +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c > @@ -19,6 +19,8 @@ > #include <media/v4l2-device.h> > #include <linux/platform_data/media/mmp-camera.h> > #include <linux/device.h> > +#include <linux/of.h> > +#include <linux/of_platform.h> > #include <linux/platform_device.h> > #include <linux/gpio.h> > #include <linux/io.h> > @@ -196,6 +198,9 @@ static void mmpcam_calc_dphy(struct mcam_camera *mcam) > struct device *dev = &cam->pdev->dev; > unsigned long tx_clk_esc; > > + if (!pdata) > + return; > + This seems a bit of an hack to me. The caller knows the mcam->bus_type and should not try to calc_dphy() if the bus type is not CSI2_DPHY. What do you think? > /* > * If CSI2_DPHY3 is calculated dynamically, > * pdata->lane_clk should be already set > @@ -314,10 +319,6 @@ static int mmpcam_probe(struct platform_device *pdev) > struct mmp_camera_platform_data *pdata; > int ret; > > - pdata = pdev->dev.platform_data; > - if (!pdata) > - return -ENODEV; > - > cam = devm_kzalloc(&pdev->dev, sizeof(*cam), GFP_KERNEL); > if (cam == NULL) > return -ENOMEM; > @@ -330,17 +331,29 @@ static int mmpcam_probe(struct platform_device *pdev) > mcam->calc_dphy = mmpcam_calc_dphy; > mcam->dev = &pdev->dev; > mcam->use_smbus = 0; > - mcam->mclk_src = pdata->mclk_src; > - mcam->mclk_div = pdata->mclk_div; > - mcam->bus_type = pdata->bus_type; > - mcam->dphy = pdata->dphy; > + pdata = pdev->dev.platform_data; > + if (pdata) { > + mcam->mclk_src = pdata->mclk_src; > + mcam->mclk_div = pdata->mclk_div; > + mcam->bus_type = pdata->bus_type; > + mcam->dphy = pdata->dphy; > + mcam->lane = pdata->lane; > + } else { > + /* > + * These are values that used to be hardcoded in mcam-core and > + * work well on a OLPC XO 1.75 with a parallel bus sensor. > + * If it turns out other setups make sense, the values should > + * be obtained from the device tree. > + */ > + mcam->mclk_src = 3; > + mcam->mclk_div = 2; I see these only used in mcam_enable_mipi(). Am I wrong? Thanks j > + } > if (mcam->bus_type == V4L2_MBUS_CSI2_DPHY) { > cam->mipi_clk = devm_clk_get(mcam->dev, "mipi"); > if ((IS_ERR(cam->mipi_clk) && mcam->dphy[2] == 0)) > return PTR_ERR(cam->mipi_clk); > } > mcam->mipi_enabled = false; > - mcam->lane = pdata->lane; > mcam->chip_id = MCAM_ARMADA610; > mcam->buffer_mode = B_DMA_sg; > strscpy(mcam->bus_info, "platform:mmp-camera", sizeof(mcam->bus_info)); > @@ -475,6 +488,10 @@ static int mmpcam_resume(struct platform_device *pdev) > > #endif > > +static const struct of_device_id mmpcam_of_match[] = { > + { .compatible = "marvell,mmp2-ccic", }, > + {}, > +}; > > static struct platform_driver mmpcam_driver = { > .probe = mmpcam_probe, > @@ -485,6 +502,7 @@ static struct platform_driver mmpcam_driver = { > #endif > .driver = { > .name = "mmp-camera", > + .of_match_table = of_match_ptr(mmpcam_of_match), > } > }; > > -- > 2.21.0 >
Ping? Lubomir: I'm applying the set now, but please addres Jacopo's comment. Thanks.
On Thu, 2019-06-20 at 14:35 +0300, Sakari Ailus wrote: > Ping? > > Lubomir: I'm applying the set now, but please addres Jacopo's comment. > Thanks. Hi, I'm wondering if you could wait a couple of minutes? I've tested the patch set on my machine last night and I was about to submit the updated set just now. Alternatively, I can just address Jacopo's comment with a patch that applies on top of the set, but it would make slightly more sense if it came earlier. Thanks Lubo
On Thu, Jun 20, 2019 at 01:37:53PM +0200, Lubomir Rintel wrote: > On Thu, 2019-06-20 at 14:35 +0300, Sakari Ailus wrote: > > Ping? > > > > Lubomir: I'm applying the set now, but please addres Jacopo's comment. > > Thanks. > > Hi, I'm wondering if you could wait a couple of minutes? > > I've tested the patch set on my machine last night and I was about to > submit the updated set just now. > > Alternatively, I can just address Jacopo's comment with a patch that > applies on top of the set, but it would make slightly more sense if it > came earlier. I applied the patches but I can always replace them with new versions --- Mauro will cherry-pick them anyway.
Hi Lubomir, Sakari, On Thu, Jun 20, 2019 at 01:37:53PM +0200, Lubomir Rintel wrote: > On Thu, 2019-06-20 at 14:35 +0300, Sakari Ailus wrote: > > Ping? > > > > Lubomir: I'm applying the set now, but please addres Jacopo's comment. > > Thanks. > > Hi, I'm wondering if you could wait a couple of minutes? > > I've tested the patch set on my machine last night and I was about to > submit the updated set just now. > > Alternatively, I can just address Jacopo's comment with a patch that > applies on top of the set, but it would make slightly more sense if it > came earlier. Indeed, sorry for the late review. I also commented on patches which are already in, so please consider if you would like to address issues on top. It's minor stuff though, nothing that seems a blocker to me. Thanks j > > Thanks > Lubo >
diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 8f1d2a972bc9..54c2dd8c29d8 100644 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -19,6 +19,8 @@ #include <media/v4l2-device.h> #include <linux/platform_data/media/mmp-camera.h> #include <linux/device.h> +#include <linux/of.h> +#include <linux/of_platform.h> #include <linux/platform_device.h> #include <linux/gpio.h> #include <linux/io.h> @@ -196,6 +198,9 @@ static void mmpcam_calc_dphy(struct mcam_camera *mcam) struct device *dev = &cam->pdev->dev; unsigned long tx_clk_esc; + if (!pdata) + return; + /* * If CSI2_DPHY3 is calculated dynamically, * pdata->lane_clk should be already set @@ -314,10 +319,6 @@ static int mmpcam_probe(struct platform_device *pdev) struct mmp_camera_platform_data *pdata; int ret; - pdata = pdev->dev.platform_data; - if (!pdata) - return -ENODEV; - cam = devm_kzalloc(&pdev->dev, sizeof(*cam), GFP_KERNEL); if (cam == NULL) return -ENOMEM; @@ -330,17 +331,29 @@ static int mmpcam_probe(struct platform_device *pdev) mcam->calc_dphy = mmpcam_calc_dphy; mcam->dev = &pdev->dev; mcam->use_smbus = 0; - mcam->mclk_src = pdata->mclk_src; - mcam->mclk_div = pdata->mclk_div; - mcam->bus_type = pdata->bus_type; - mcam->dphy = pdata->dphy; + pdata = pdev->dev.platform_data; + if (pdata) { + mcam->mclk_src = pdata->mclk_src; + mcam->mclk_div = pdata->mclk_div; + mcam->bus_type = pdata->bus_type; + mcam->dphy = pdata->dphy; + mcam->lane = pdata->lane; + } else { + /* + * These are values that used to be hardcoded in mcam-core and + * work well on a OLPC XO 1.75 with a parallel bus sensor. + * If it turns out other setups make sense, the values should + * be obtained from the device tree. + */ + mcam->mclk_src = 3; + mcam->mclk_div = 2; + } if (mcam->bus_type == V4L2_MBUS_CSI2_DPHY) { cam->mipi_clk = devm_clk_get(mcam->dev, "mipi"); if ((IS_ERR(cam->mipi_clk) && mcam->dphy[2] == 0)) return PTR_ERR(cam->mipi_clk); } mcam->mipi_enabled = false; - mcam->lane = pdata->lane; mcam->chip_id = MCAM_ARMADA610; mcam->buffer_mode = B_DMA_sg; strscpy(mcam->bus_info, "platform:mmp-camera", sizeof(mcam->bus_info)); @@ -475,6 +488,10 @@ static int mmpcam_resume(struct platform_device *pdev) #endif +static const struct of_device_id mmpcam_of_match[] = { + { .compatible = "marvell,mmp2-ccic", }, + {}, +}; static struct platform_driver mmpcam_driver = { .probe = mmpcam_probe, @@ -485,6 +502,7 @@ static struct platform_driver mmpcam_driver = { #endif .driver = { .name = "mmp-camera", + .of_match_table = of_match_ptr(mmpcam_of_match), } };