From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Matti Vaittinen <mazziesaccount@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
Mehdi Djait <mehdi.djait.k@gmail.com>,
krzysztof.kozlowski+dt@linaro.org, robh+dt@kernel.org,
lars@metafoo.de, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v8 6/7] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer
Date: Mon, 11 Sep 2023 16:03:05 +0300 [thread overview]
Message-ID: <ZP8QCVbOpUOMxV8g@smile.fi.intel.com> (raw)
In-Reply-To: <8b73d2d9-ebba-8f3c-4b39-e3671117acf1@gmail.com>
On Thu, Sep 07, 2023 at 09:33:47AM +0300, Matti Vaittinen wrote:
> On 9/6/23 19:03, Andy Shevchenko wrote:
> > On Tue, Aug 29, 2023 at 09:33:27AM +0300, Matti Vaittinen wrote:
> > > On 8/28/23 13:53, Andy Shevchenko wrote:
> > > > On Mon, Aug 28, 2023 at 09:24:25AM +0300, Matti Vaittinen wrote:
> > > > > On 8/27/23 21:09, Jonathan Cameron wrote:
...
> > > > > I think that people who work on a driver like this should guess what this is
> > > > > for.
> > > >
> > > > _This_ is the result of what people always forgot to think about, i.e.
> > > > newcomers.
> > >
> > > Thanks Andy. This was a good heads-up for me. I do also see the need for
> > > fresh blood here - we aren't getting any younger.
> > >
> > > > What _if_ the newcomer starts with this code and already being puzzled
> > > > enough on what the heck the function does. With all ambiguity we rise
> > > > the threshold for the newcomers and make the kernel project not
> > > > attractive to start with
> > >
> > > I really appreciate you making a point about attracting newcomers (and there
> > > is no sarcasm in this statement). I however don't think we're rising the bar
> > > here. If a newcomer wants to work on a device-driver, the _first_ thing to
> > > do is to be familiar with the device. Without prior experience of this kind
> > > of devices it is really a must to get the data-sheet and see how the device
> > > operates before jumping into reading the code. I would say that after
> > > reading the fifo lvl description from data-sheet this should be obvious -
> > > and no, I don't think we should replicate the data-sheet documentation in
> > > the drivers for parts that aren't very peculiar.
> >
> > There are (at least?) two approaches on the contribution:
> > 1) generic / library wise;
> > 2) specific hardware wise.
> >
> > You are talking about 2), while my remark is about both. I can imagine a
> > newcomer who possess a hardware that looks similar to what this driver is
> > for.
>
> Yes. I am talking about 2). And my stance is that device drivers belong to
> category 2). If one works with a device driver for some HW, then he/she
> needs to be willing to understand the hardware.
>
> > Now, they would like to write a new driver (note, that compatibility can be
> > checked by reading the RTL definitions, so no need to dive into the code)
> > and use this as a (nice) reference. With that in mind, they can read a
> > function named get_fifo_bytes() with not so extensive documentation nor
> > fully self-explanatory name. One may mistakenly though about this as a
> > function for something that returns FIFO capacity, but in the reality it is
> > current amount of valid / data bytes in the FIFO for the ongoing
> > communication with the device.
>
> I can't avoid having a feeling that this is a very unlikely scenario. I am
> afraid that by requesting this type of improvements at patch series which is
> at v8 and has been running for half an year (and which was of a good quality
> to start with, especially knowing this was the author's first driver) is
> going to be more repulsive to the newcomers than the potential obfuscation.
I agree and this is a side talk to the topic.
> I don't try claiming that no-one could ever hit this trap (even if I don't
> see it likely). I still believe that if one does so, he/she will also get
> such a bug fixed without being totally discouraged - it's business as usual.
>
> I hope this does not come out as rude. I do appreciate your reviews, it's
> comforting to know someone looks my code with sharp eyes and points out
> things like the dead code in BM1390 driver! I just like the words Jonathan
> once spilled out:
>
> "Don't let the perfect be enemy of good" (or something along those lines).
True.
> > > But the question how to attract newcomers to kernel is very valid and I
> > > guess that not too many of us is thinking of it. Actually, I think we should
> > > ask from the newcomers we have that what has been the most repulsive part of
> > > the work when they have contributed.
> >
> > > > (besides the C language which is already considered as mastodon among
> > > > youngsters).
> > >
> > > I think this is at least partially the truth. However, I think that in many
> > > cases one of the issues goes beyond the language - many younger generation
> > > people I know aren't really interested in _why_ things work, they just want
> > > to get things working in any way they can - and nowadays when you can find a
> > > tutorial for pretty much anything - one really can just look up instruction
> > > about how a "foobar can be made to buzz" instead of trying to figure out
> > > what makes a "foobar to buzz" in order to make it to buzz. So, I don't blame
> > > people getting used to take a different approach. (Not sure this makes sense
> > > - don't really know how to express my thoughts about this in a clear way -
> > > besides, it may not even matter).
> >
> > Yeah, I share your frustration and agree that people are loosing the feel of
> > curiosity. Brave New World in front of us...
>
> Well, who knows how things will be working out for the new generations?
> Maybe they won't need the kernel in the future? Yes, I am stubbornly hanging
> in the past practices and values. Direction things seem to head do not
> always appeal to me - but perhaps it's just me? Who can say my values and
> practices are the right ones for new generations :) My oldest son just moved
> to his own home and I need to accept that young do build their own lives on
> different values I had. And who knows, maybe the approach of just doing
> things without knowing what exactly happens under the hood makes this world
> very good for them?
>
> But yes - I don't think it suits the kernel project at all :) This is a
> project of dinosaurs like us XD
>
> (DISCLAIMER: I don't know quite all young people in the world. Frankly to
> tell, not even 90% XD So, I am not trying to say "all young people are like
> this or that".
Operating in terms of universal quantifier is always wrong (pun intended).
> I just have a feeling that certain way of thinking is more
> common amongst certain generations - but maybe it's just my misjudgement.
> Please, don't be offended).
>
> > > Anyways, I am pretty sure that - as with any community - the way people
> > > are treated and how their contribution is appreciated is the key to make
> > > them feel good and like the work. I think that in some cases it may
> > > include allowing new contributors to get their code merged when it has
> > > reached "good enough" state - even if it was not perfect. (Sure, when
> > > things are good enough is subject to greater minds than me to ponder) ;)
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2023-09-11 21:58 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-23 21:16 [PATCH v8 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
2023-08-23 21:16 ` [PATCH v8 1/7] dt-bindings: iio: Add " Mehdi Djait
2023-08-23 21:16 ` [PATCH v8 2/7] iio: accel: kionix-kx022a: Remove blank lines Mehdi Djait
2023-08-24 11:49 ` Andy Shevchenko
2023-08-23 21:16 ` [PATCH v8 3/7] iio: accel: kionix-kx022a: Warn on failed matches and assume compatibility Mehdi Djait
2023-08-24 11:51 ` Andy Shevchenko
2023-08-27 17:57 ` Jonathan Cameron
2023-08-23 21:16 ` [PATCH v8 4/7] iio: accel: kionix-kx022a: Add an i2c_device_id table Mehdi Djait
2023-08-24 11:52 ` Andy Shevchenko
2023-08-23 21:16 ` [PATCH v8 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure Mehdi Djait
2023-08-24 11:55 ` Andy Shevchenko
2023-08-23 21:16 ` [PATCH v8 6/7] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer Mehdi Djait
2023-08-24 11:58 ` Andy Shevchenko
2023-08-24 12:52 ` Matti Vaittinen
2023-08-24 13:39 ` Andy Shevchenko
2023-08-24 13:44 ` Mehdi Djait
2023-08-24 13:47 ` Andy Shevchenko
2023-08-24 14:23 ` Mehdi Djait
2023-08-24 14:39 ` Andy Shevchenko
2023-08-27 18:09 ` Jonathan Cameron
2023-08-28 6:24 ` Matti Vaittinen
2023-08-28 10:53 ` Andy Shevchenko
2023-08-29 6:33 ` Matti Vaittinen
2023-09-06 16:03 ` Andy Shevchenko
2023-09-07 6:33 ` Matti Vaittinen
2023-09-11 13:03 ` Andy Shevchenko [this message]
2023-08-23 21:16 ` [PATCH v8 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
2023-08-24 12:02 ` Andy Shevchenko
2023-08-24 12:51 ` Matti Vaittinen
2023-08-24 13:39 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZP8QCVbOpUOMxV8g@smile.fi.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=devicetree@vger.kernel.org \
--cc=jic23@kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mazziesaccount@gmail.com \
--cc=mehdi.djait.k@gmail.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox