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);
>
> [...]
