mbox series

[0/2] media: imx-mipi-csis: csis clock fixes

Message ID 20231122-imx-csis-v1-0-0617368eb996@ideasonboard.com
Headers
Series media: imx-mipi-csis: csis clock fixes |

Message

Tomi Valkeinen Nov. 22, 2023, 1:13 p.m. UTC 
  Two fixes to the csis driver: One to fix remove() another to only enable
the clocks when needed.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Tomi Valkeinen (2):
      media: imx-mipi-csis: Fix clock handling in remove()
      media: imx-mipi-csis: Drop extra clock enable at probe()

 drivers/media/platform/nxp/imx-mipi-csis.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)
---
base-commit: 1865913dd590ca6d5e3207b15099a1210dd62f29
change-id: 20231122-imx-csis-79d201dd51b9

Best regards,

Comments

Fabio Estevam Nov. 22, 2023, 1:21 p.m. UTC | #1 
Hi Tomi,

On Wed, Nov 22, 2023 at 10:14 AM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
>
> Two fixes to the csis driver: One to fix remove() another to only enable
> the clocks when needed.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> Tomi Valkeinen (2):
>       media: imx-mipi-csis: Fix clock handling in remove()
>       media: imx-mipi-csis: Drop extra clock enable at probe()

Shouldn't both patches contain a Fixes tag?
Tomi Valkeinen Nov. 22, 2023, 1:22 p.m. UTC | #2 
On 22/11/2023 15:21, Fabio Estevam wrote:
> Hi Tomi,
> 
> On Wed, Nov 22, 2023 at 10:14 AM Tomi Valkeinen
> <tomi.valkeinen@ideasonboard.com> wrote:
>>
>> Two fixes to the csis driver: One to fix remove() another to only enable
>> the clocks when needed.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> Tomi Valkeinen (2):
>>        media: imx-mipi-csis: Fix clock handling in remove()
>>        media: imx-mipi-csis: Drop extra clock enable at probe()
> 
> Shouldn't both patches contain a Fixes tag?

Indeed, yes, I'll add that.

  Tomi
Tomi Valkeinen Nov. 22, 2023, 1:44 p.m. UTC | #3 
On 22/11/2023 15:21, Fabio Estevam wrote:
> Hi Tomi,
> 
> On Wed, Nov 22, 2023 at 10:14 AM Tomi Valkeinen
> <tomi.valkeinen@ideasonboard.com> wrote:
>>
>> Two fixes to the csis driver: One to fix remove() another to only enable
>> the clocks when needed.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> Tomi Valkeinen (2):
>>        media: imx-mipi-csis: Fix clock handling in remove()
>>        media: imx-mipi-csis: Drop extra clock enable at probe()
> 
> Shouldn't both patches contain a Fixes tag?

I think the issue is there in the original commit adding the driver:

7807063b862b ("media: staging/imx7: add MIPI CSI-2 receiver subdev for 
i.MX7")

However, the driver has changed along the way, and I'm not sure if the 
original one had an actual bug. Nevertheless, the same pattern (wrt. 
clocks and runtime) is there in the original one, and I think that 
pattern is not correct even if it wouldn't have caused any visible issue.

So I'll add that commit as Fixes-tag, but if someone with more knowledge 
about the driver can verify this, that'd be great.

  Tomi
Laurent Pinchart Nov. 22, 2023, 3:05 p.m. UTC | #4 
On Wed, Nov 22, 2023 at 03:44:33PM +0200, Tomi Valkeinen wrote:
> On 22/11/2023 15:21, Fabio Estevam wrote:
> > On Wed, Nov 22, 2023 at 10:14 AM Tomi Valkeinen wrote:
> >>
> >> Two fixes to the csis driver: One to fix remove() another to only enable
> >> the clocks when needed.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >> ---
> >> Tomi Valkeinen (2):
> >>        media: imx-mipi-csis: Fix clock handling in remove()
> >>        media: imx-mipi-csis: Drop extra clock enable at probe()
> > 
> > Shouldn't both patches contain a Fixes tag?
> 
> I think the issue is there in the original commit adding the driver:
> 
> 7807063b862b ("media: staging/imx7: add MIPI CSI-2 receiver subdev for 
> i.MX7")
> 
> However, the driver has changed along the way, and I'm not sure if the 
> original one had an actual bug. Nevertheless, the same pattern (wrt. 
> clocks and runtime) is there in the original one, and I think that 
> pattern is not correct even if it wouldn't have caused any visible issue.
> 
> So I'll add that commit as Fixes-tag, but if someone with more knowledge 
> about the driver can verify this, that'd be great.

Sounds fine to me. I assume you'll send a v2.