Message-ID: <20191115092745.GA21025@codeaurora.org>
Date:   Fri, 15 Nov 2019 01:27:45 -0800
From:   Guru Das Srinagesh <gurus@...eaurora.org>
To:     Thierry Reding <thierry.reding@...il.com>
Cc:     linux-pwm@...r.kernel.org, kernel-team@...roid.com,
        Mark Salyzyn <salyzyn@...gle.com>,
        Sandeep Patil <sspatil@...gle.com>,
        Subbaraman Narayanamurthy <subbaram@...eaurora.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] pwm: Convert period and duty cycle to u64

Hi Thierry,

On Thu, Oct 17, 2019 at 12:43:13PM +0200, Thierry Reding wrote:
> On Wed, Oct 16, 2019 at 11:02:47PM -0700, Guru Das Srinagesh wrote:
> > On Wed, Oct 16, 2019 at 12:15:39PM +0200, Thierry Reding wrote:
> > > On Tue, Oct 15, 2019 at 07:11:39PM -0700, Guru Das Srinagesh wrote:
> > > > Because period and duty cycle are defined as ints with units of
> > > > nanoseconds, the maximum time duration that can be set is limited to
> > > > ~2.147 seconds. Change their definitions to u64 so that higher durations
> > > > may be set.
> > > > 
> > > > Signed-off-by: Guru Das Srinagesh <gurus@...eaurora.org>
> > > > ---
> > > >  drivers/pwm/core.c  |  4 ++--
> > > >  drivers/pwm/sysfs.c | 10 +++++-----
> > > >  include/linux/pwm.h | 16 ++++++++--------
> > > >  3 files changed, 15 insertions(+), 15 deletions(-)
> > > 
> > > Actually, we can't do that without further preparatory work. The reason
> > > is that consumers use the period and duty_cycle members in computations
> > > of their own, which lead to errors such as this:
> > > 
> > > 	armv7l-unknown-linux-gnueabihf-ld: drivers/video/backlight/pwm_bl.o: in function `pwm_backlight_probe':
> > > 	pwm_bl.c:(.text+0x3b0): undefined reference to `__aeabi_uldivmod'
> > > 
> > > So I think we need to audit all consumers carefully and make sure that
> > > they use do_div() where necessary to avoid such errors.
> > > 
> > > Thierry
> > 
> > Hi Thierry,
> > 
> > I would like to try doing the preparatory work by fixing the errors seen
> > in consumers so that this u64 patch may be applied without issues.
> > 
> > Before sending the patch, I tried "make"-ing for arm, arm64 and i386
> > architectures to check for compilation/linking errors and encountered
> > none. I see that the above error arises from using a cross-compiler for
> > arm v7, which I haven't tried yet.
> > 
> > Could you please provide details of the compile tests that you run at
> > your end? I could then try to reproduce the errors you see in the
> > consumer drivers and fix them. Please do share any other ideas or
> > suggestions you may have in this regard.
> 
> I keep a set of scripts in the pwm/ subdirectory of the following
> repository:
> 
> 	https://github.com/thierryreding/scripts
> 
> Typically what I do is run:
> 
> 	$ /path/to/scripts.git/pwm/build --jobs 13 --color
> 
> That requires a bit of setup for the cross-compilers. I have the
> following in my ~/.cross-compile file:
> 
> 	path: $HOME/pbs-stage1/bin:$HOME/toolchain/avr32/bin:$HOME/toolchain/unicore32/bin
> 	arm: armv7l-unknown-linux-gnueabihf-
> 	arm64: aarch64-unknown-linux-gnu-
> 	avr32: avr32-
> 	blackfin: bfin-unknown-elf-
> 	mips: mips-linux-gnu-
> 	unicore32: unicore32-linux-
> 	riscv: riscv64-linux-gnu-
> 	x86:
> 	x86_64:
> 
> The blackfin and unicore32 builds are expected to fail because the
> blackfin architecture was removed and there's no recent enough kernel
> publicly available for unicore32.
> 
> The last two entries in .cross-compile indicate that builds are native,
> so regular gcc from the build system will be used.
> 
> Most of these compilers I've built from scratch using pbs-stage1:
> 
> 	https://github.com/thierryreding/pbs-stage1
> 
> Note that I don't guarantee that that build system works for anyone but
> myself, but I'd be happy to hear feedback if you decide to use it. That
> said, you can probably find prebuilt toolchains for all of the above in
> a number of locations, like:
> 
> 	https://mirrors.edge.kernel.org/pub/tools/crosstool/
> 
> or:
> 
> 	https://toolchains.bootlin.com/
> 
> Thierry

I tried replicating your compilation setup and found that it worked
right out of the box with no serious issues. I decided to build the
compilers from scratch and only had to update my make to the latest
version and also install help2man and u-boot-tools on my Ubuntu machine.

I found your setup very easy to use on the whole and very well designed.
I added a "set -x" to the /path/to/scripts.git/build/pwm script in order
to figure out how it worked initially. 

I didn't add the lines for unicore32 and blackfin in my ~/.cross-compile
file as you had indicated that they were expected to fail. It was very
convenient for me to run the build command for a specific arch by
appending its name to the end of the command you provided and thus
verify that the compilation errors I was getting were getting fixed for
that arch. Then by simply dropping the architecture's name from the end
I could run the build command for all archs - very cool.

That said, I wasn't able to compile-test avr32 and x86_64. I couldn't
find avr32 in the targets folder of the pbs-stage1 git repo and so
couldn't build it from scratch, and I couldn't find a pre-built version
either. There isn't a config for x86_64 in
/path/to/scripts.git/pwm/configs and so the build/pwm script wasn't
picking it up even though I had added a blank line for it in
~/.cross-compile. Those are the only two issues I encountered with
replicating your setup.

Thank you.

Guru Das.

Powered by blists - more mailing lists