| Message ID | BN6PR04MB06604D29C9F66EB53FB17581A3AE0@BN6PR04MB0660.namprd04.prod.outlook.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | media: exynos4-is: Improve support for s5pv210 and parallel ports | expand |
Hi Jonathan, On Sat, Apr 25, 2020 at 07:26:44PM -0700, Jonathan Bakker wrote: > Commit 1c9f5bd7cb8a ("[media] s5p-fimc: Add support for sensors with > multiple pads") caught the case where a sensor with multiple pads was > connected via CSIS, but missed the case where the sensor was directly > connected to the FIMC. > > This still assumes that the last pad of a sensor is the source. > > Signed-off-by: Jonathan Bakker <xc-racer2@live.ca> > --- > drivers/media/platform/exynos4-is/media-dev.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Thank you for the patch. Please see my comments inline. > diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c > index 5c32abc7251b..b38445219c72 100644 > --- a/drivers/media/platform/exynos4-is/media-dev.c > +++ b/drivers/media/platform/exynos4-is/media-dev.c > @@ -991,7 +991,8 @@ static int fimc_md_create_links(struct fimc_md *fmd) > > case FIMC_BUS_TYPE_ITU_601...FIMC_BUS_TYPE_ITU_656: > source = &sensor->entity; > - pad = 0; > + /* Assume the last pad is the source */ > + pad = sensor->entity.num_pads - 1; Is 0 really any worse than num_pads - 1? This sounds like quite an ugly hack. Could you iterate over the pads of the sensor entity and explicitly find a source pad instead? Best regards, Tomasz
Hi Tomasz, On 2020-07-07 11:13 a.m., Tomasz Figa wrote: > Hi Jonathan, > > On Sat, Apr 25, 2020 at 07:26:44PM -0700, Jonathan Bakker wrote: >> Commit 1c9f5bd7cb8a ("[media] s5p-fimc: Add support for sensors with >> multiple pads") caught the case where a sensor with multiple pads was >> connected via CSIS, but missed the case where the sensor was directly >> connected to the FIMC. >> >> This still assumes that the last pad of a sensor is the source. >> >> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca> >> --- >> drivers/media/platform/exynos4-is/media-dev.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> > > Thank you for the patch. Please see my comments inline. > >> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c >> index 5c32abc7251b..b38445219c72 100644 >> --- a/drivers/media/platform/exynos4-is/media-dev.c >> +++ b/drivers/media/platform/exynos4-is/media-dev.c >> @@ -991,7 +991,8 @@ static int fimc_md_create_links(struct fimc_md *fmd) >> >> case FIMC_BUS_TYPE_ITU_601...FIMC_BUS_TYPE_ITU_656: >> source = &sensor->entity; >> - pad = 0; >> + /* Assume the last pad is the source */ >> + pad = sensor->entity.num_pads - 1; > > Is 0 really any worse than num_pads - 1? This sounds like quite an ugly > hack. > > Could you iterate over the pads of the sensor entity and explicitly find > a source pad instead? Yes, iterating would work better. This comes from when I was trying to integrate video-mux, before I realized I could connect multiple sensors. I will drop this patch from v2 as it's not necessary. > > Best regards, > Tomasz > Thanks, Jonathan
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c index 5c32abc7251b..b38445219c72 100644 --- a/drivers/media/platform/exynos4-is/media-dev.c +++ b/drivers/media/platform/exynos4-is/media-dev.c @@ -991,7 +991,8 @@ static int fimc_md_create_links(struct fimc_md *fmd) case FIMC_BUS_TYPE_ITU_601...FIMC_BUS_TYPE_ITU_656: source = &sensor->entity; - pad = 0; + /* Assume the last pad is the source */ + pad = sensor->entity.num_pads - 1; break; default:
Commit 1c9f5bd7cb8a ("[media] s5p-fimc: Add support for sensors with multiple pads") caught the case where a sensor with multiple pads was connected via CSIS, but missed the case where the sensor was directly connected to the FIMC. This still assumes that the last pad of a sensor is the source. Signed-off-by: Jonathan Bakker <xc-racer2@live.ca> --- drivers/media/platform/exynos4-is/media-dev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)