On Mon, 2023-11-27 at 09:12 +0100, Krzysztof Kozlowski wrote:
> On 27/11/2023 09:10, Krzysztof Kozlowski wrote:
> > On 27/11/2023 08:53, Nuno Sá wrote:
> > > > 355
> > > > > 356 unsigned long ltc4282_recalc_rate(struct clk_hw *hw, unsigned long
> > > > parent)
> > > > 357 {
> > > > 358 struct ltc4282_state *st = container_of(hw, struct
> > > > ltc4282_state,
> > > > 359 clk_hw);
> > > > 360 u32 clkdiv;
> > > > 361 int ret;
> > > > 362
> > > > 363 ret = regmap_read(st->map, LTC4282_CLK_DIV, &clkdiv);
> > > > 364 if (ret)
> > > > 365 return 0;
> > > > 366
> > > > 367 clkdiv = FIELD_GET(LTC4282_CLKOUT_MASK, clkdiv);
> > > > 368 if (!clkdiv)
> > > > 369 return 0;
> > > > 370 if (clkdiv == LTC4282_CLKOUT_INT)
> > > > 371 return LTC4282_CLKOUT_SYSTEM;
> > > > 372
> > > > 373 return LTC4282_CLKOUT_CNV;
> > > > 374 }
> > > > 375
> > > >
> > >
> > > Arghh, I do need to see if I can add some test branch of my own to the test
> > > robot :/.
> > > Anyways, will wait for some more reviewing before sending v3 to address this.
> >
> > It's easy to test your patches on your own machines... Just build few
> > different configs.
> >
>
> Wait, this was not even unusual test, just standard compile, which means
> you did not do basic tests on your end. You must build your new driver
> with W=1, smatch, sparse and coccinelle before sending upstream.
>
> Best regards,
> Krzysztof
>
Well, I do agree that a sparse build would easily caught this and I'm mad with myself
because that much (together with checkpatch) I usually run (I confess that coccinelle
and smatch is something that I don't usually do.
"you did not do basic tests on your end"
However I do think this is a bit unfair. I did got a lot of things to do in v2 with
major refactors on some of the features. So "basic test" to me is actually making
sure that all of these changes don't break the driver and the device still works as
expected.
Anyways, I don't want to find any execuses and start an argument. Will try to do
better in next iterations.
- Nuno Sá
