Thread (15 messages) 15 messages, 5 authors, 2015-01-30

Re: [PATCH v8 0/2] Imagination Technologies PWM support

From: Thierry Reding <hidden>
Date: 2015-01-30 11:26:11
Also in: linux-pwm, lkml

On Fri, Jan 30, 2015 at 08:01:00AM -0300, Ezequiel Garcia wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi Thierry,

On 01/30/2015 06:18 AM, Thierry Reding wrote:
quoted
On Thu, Jan 29, 2015 at 01:32:09PM -0300, Ezequiel Garcia wrote:
quoted
On 01/20/2015 07:52 AM, Ezequiel Garcia wrote:
quoted

On 01/09/2015 02:54 PM, Ezequiel Garcia wrote:
quoted
A new round for the IMG PWM driver.

The IMG PWM controller is muxed with a PDM controller,
through a shared so-called periph register bit, which sets
the output as PWM or PDM. Because this register is not part
of the pin controller block, but rather PWM/PDM specific, and
because the register is also used to set the PDM value, it is
simpler to use a regmap-based syscon to deal with it.

This time, I'm removing the PDM driver from the submission
and submitting the PWM alone. The PDM was written as a misc
driver, but had some design issues, so for now I'm proposing
to merge the PWM only.

The series is based on v3.19-rc3. If at all possible I'd like
to see this merged for v3.20.
Thierry,

Any comments on this? Any chance we merge it in time for
v3.20? It's -rc5 already and I've started to worry.
Thierry,

I'm very sorry to be so bothering, but I got no news from you and
it's -rc6 already. I'd say I'll resend this series to Andrew
Morton, hoping we can merge this for v3.20.

Please let me know if you'll be able to pick this.
I can pick it up if you make up your mind about the license. The
header comment says GPL v2 or later, but MODULE_LICENSE has "GPL
v2", which does not include the "or later" part.
License should be GPL v2, sorry about that. Need me to fix it and
resend or can you amend it before pushing this?
I've fixed it up.
quoted
Also you're making it especially difficult to build-test by not 
providing even the basic bits of your SoC support first. All even 
linux-next seems to have for the Pistachio SoC is the addition of
a compatible string to the dw-mmc driver.
Well, we've added COMPILE_TEST for precisely this reason, so you only
need to select COMPILE_TEST on any arch and you'll be able to build
test the driver.
I'm not too big a fan of COMPILE_TEST. Sometimes build errors happen
only on one architecture, so if I build test your driver for ARM I may
miss issues and people will later complain to me that my tree broke
their build. So I prefer building on the target architecture if at all
possible.
quoted
I'll take the PWM driver, but I'll assume that you'll eventually
have more pieces available, in which case I'd appreciate a note so
I can update my build scripts.
If you can pick it, it would be great. I understand it's hard to
accept patches for drivers, when there's little testing possibilities.
But on the other side, isn't it a positive thing that a vendor is
pushing drivers this early?
I wonder what's keeping the basic SoC support from being merged. As it
is the driver is completely unused, so I'm merging dead code. I'm going
to trust Andrew when he says that he plans on merging the SoC bits soon
so it's okay for now.

The problem with merging drivers first is that you have no possibility
of testing them in an upstream environment. You can't even test them
yourself even if you have local patches to make things actually boot.
I've seen that lead to unnecessary churn later on.

Yes, vendors contributing early is a good thing, but submitting code
that can't be tested in an upstream environment isn't very useful. You
can never be entirely certain that it's going to work before you have
the basic SoC support merged first.

Thierry

Attachments

  • (unnamed) [application/pgp-signature] 819 bytes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help