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

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

Hello Marek,

Am Dienstag, 10. Januar 2023, 14:37:19 CET schrieb Marek Vasut:
> On 1/10/23 14:22, Alexander Stein wrote:
> > Hi Marek,
> 
> Hi,
> 
> > 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?
> 
> Good point, but then, what's the point of ORRing either ? Just do a
> plain assignment.

OR-ring is necessary as this function is called for each DIF output (see idx 
parameter), so plain assignment will clear the previously set bits.

Best regards,
Alexander