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
