Re: [PATCH 2/2] media: i2c: ov13b10: Support 2 lane mode

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Hao,

On Tue, Mar 11, 2025 at 02:42:57PM +0800, Hao Yao wrote:
> Hi Sakari,
> 
> On 2025/3/10 15:14, Sakari Ailus wrote:
> > Hi Hao,
> > 
> > On Mon, Mar 10, 2025 at 12:06:04PM +0800, Hao Yao wrote:
> > > > > @@ -1459,12 +1539,25 @@ static int ov13b10_check_hwcfg(struct device *dev)
> > > > >    	if (ret)
> > > > >    		return ret;
> > > > > -	if (bus_cfg.bus.mipi_csi2.num_data_lanes != OV13B10_DATA_LANES) {
> > > > > +	dlane = bus_cfg.bus.mipi_csi2.num_data_lanes;
> > > > > +	if (dlane != OV13B10_4_DATA_LANES && dlane != OV13B10_2_DATA_LANES) {
> > > > >    		dev_err(dev, "number of CSI2 data lanes %d is not supported",
> > > > > -			bus_cfg.bus.mipi_csi2.num_data_lanes);
> > > > > +			dlane);
> > > > >    		ret = -EINVAL;
> > > > >    		goto out_err;
> > > > >    	}
> > > > > +	ov13b->data_lanes = dlane;
> > > > > +	ov13b->supported_modes = supported_4_lanes_modes;
> > > > > +	ov13b->supported_modes_num = ARRAY_SIZE(supported_4_lanes_modes);
> > > > > +	if (dlane == OV13B10_2_DATA_LANES) {
> > > > > +		ov13b->supported_modes = supported_2_lanes_modes;
> > > > > +		ov13b->supported_modes_num =
> > > > > +			ARRAY_SIZE(supported_2_lanes_modes);
> > > > 
> > > > How about using switch() here?
> > > 
> > > How about:
> > > 	ov13b->data_lanes = bus_cfg.bus.mipi_csi2.num_data_lanes;
> > > 	switch (ov13b->data_lanes) {
> > > 	case OV13B10_4_DATA_LANES:
> > > 		ov13b->supported_modes = supported_4_lanes_modes;
> > > 		ov13b->supported_modes_num =
> > > 			ARRAY_SIZE(supported_4_lanes_modes);
> > > 		break;
> > > 
> > > 	case OV13B10_2_DATA_LANES:
> > > 		ov13b->supported_modes = supported_2_lanes_modes;
> > > 		ov13b->supported_modes_num =
> > > 			ARRAY_SIZE(supported_2_lanes_modes);
> > > 		break;
> > > 
> > > 	default:
> > > 		dev_err(dev, "number of CSI2 data lanes %d is not supported",
> > > 			ov13b->data_lanes);
> > > 		ret = -EINVAL;
> > > 		goto out_err;
> > > 	}
> > 
> > Looks good to me. I'd do the assignment to ov13b->data_lanes after checking
> > the value though.
> > 
> 
> Thank you, I will prepare v2 later.
> BTW, do you have any suggestions on patch 1/2 ?

That looks good to me.

-- 
Sakari Ailus