[PATCH 0/1][SRU][F] Dynamically set AD5593R channel modes

[AceLan Kao]

Sorry, I missed this email.
Let me answer your questions below

Stefan Bader <stefan.bader at canonical.com> 於 2020年10月13日 週二 下午5:08寫道：
>
> On 15.09.20 07:08, AceLan Kao wrote:
> > BugLink: https://bugs.launchpad.net/bugs/1895612
> >
> > [Impact]
> > On Advantech UNO-420 development platform, we need a generic way to set
> > the pin mode.
> > Upstream would like us to leverage ACPI override method to change the
> > modes, but in Ubuntu core, we can't generate a new initramfs and boot up
> > with the generated initramfs.
> >
> > [Fix]
> > Add a module parameter to assign pin modes while loading ad5593r driver.
> >
> > [Test]
> > Verified on Advantech UNO-420 platform.
> >
> > [Regression Potential]
> > Low, it checks the length of the passed parameter, and checks every bytes
> > in it to make sure it's a valid argument.
> >
> > William Sung (1):
> >   iio: dac: ad5593r: Dynamically set AD5593R channel modes
> >
> >  drivers/iio/dac/ad5592r-base.c | 21 +++++++++++++---
> >  drivers/iio/dac/ad5592r-base.h |  4 ++++
> >  drivers/iio/dac/ad5593r.c      | 44 ++++++++++++++++++++++++++++++++++
> >  3 files changed, 66 insertions(+), 3 deletions(-)
> >
>
> I did discuss this with Andy today. Technically I guess this is ok if we are
> sure that you cannot damage the HW or leverage the interface for attacks by
> passing either random or certain sequences of numbers. The allowed values seem
> to be checked, so the question only is what one could do with random
> alternations of those.
The pin modes are for the devices connected to those pins, so it
should not break the system,
but maybe lead to the devices unable to work.

>
> The other thing we want to avoid is to have a module argument name either clash
> with a possible upstream name or be taken as something that will be carried
> onward into later series.
>
> So question: could we prefix the paramenter with ubuntu, so it would become
> ubuntu_ch_mode=... Then maybe we could add Documentation/admin-guide/
> ubuntu-kernel-parameters.txt which explains the usage and also clarifies that
> this is an experimental option which may or (rather) may not be present in newer
> kernels.
I can't tell if this is a good idea.
I agree we should have some notes somewhere to describe the patches' life cycle,
maybe written it on patch description directly, like ODM: 5.4, to
specify this patch is from ODM and stay in 5.4 only.
And for the parameter rename, I don't think it's necessary. Upstream
had rejected this patch,
so there should be no parameter name conflict in the future.
And the parameter is for a limited group of people who already know
how to use it,
and should know whom to complain if this parameter is gone. So, create
an a document for them is not so important.
>
> -Stefan
>