Re: [PATCH v2 3/4] clk: rs9: Support device specific dif bit calculation

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hi Marek,

thanks for your feedback.

Am Dienstag, 10. Januar 2023, 11:31:49 CET schrieb Marek Vasut:
> On 1/10/23 11:00, Alexander Stein wrote:
> 
> [...]
> 
> >   static int rs9_get_output_config(struct rs9_driver_data *rs9, int idx)
> >   {
> >   
> >   	struct i2c_client *client = rs9->client;
> > 
> > +	u8 dif = rs9_calc_dif(rs9, idx);
> > 
> >   	unsigned char name[5] = "DIF0";
> >   	struct device_node *np;
> >   	int ret;
> >   	u32 sr;
> >   	
> >   	/* Set defaults */
> > 
> > -	rs9->clk_dif_sr &= ~RS9_REG_SR_DIF_MASK(idx);
> 
> Are you sure this line ^ should be dropped ?
> Shouldn't the bitfield be cleared first and modified second?

Well, I had in my mind that this function is called upon probe with clk_dif_sr 
being cleared anyway, so this does essentially nothing. And the DIF bit is set 
unconditionally, so what is the point of masking it before?

Best regards,
Alexander

> > -	rs9->clk_dif_sr |= RS9_REG_SR_3V0_DIF(idx);
> > +	rs9->clk_dif_sr |= dif;
> > 
> >   	snprintf(name, 5, "DIF%d", idx);
> >   	np = of_get_child_by_name(client->dev.of_node, name);
> 
> [...]