Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <X863KNo0IaekkU7q@workstation.tuxnet> <20201208091033.bxzrlad7mjbe3dsp@pengutronix.de>
 <X89RgpTb3sBBI++w@workstation.tuxnet> <X8+DI7ZN7mXtsxv9@ulmo>
 <CAGngYiXgVbEXj-yR=DTeA4pO-N3=WhiHjQhknFsbfXBeD_yRbw@mail.gmail.com>
 <X8+waLH58pOaMI06@ulmo> <20201208182637.hm5uzuw5ueelo26k@pengutronix.de>
 <X9EDGHySNYb7CxcW@ulmo> <20201210090124.rfswkrcttsg5gszp@pengutronix.de>
 <X9JWlVPb9ZGdB4q9@ulmo> <X9KK9UrXu+b8F+IT@workstation.tuxnet>
In-Reply-To: <X9KK9UrXu+b8F+IT@workstation.tuxnet>
From:   Sven Van Asbroeck <thesven73@gmail.com>
Date:   Thu, 10 Dec 2020 16:37:01 -0500
Message-ID: <CAGngYiVAKraGxOYe5bCNhzhPSvFMh5u7cZ-ENqzTv4kvbNjryw@mail.gmail.com>
Subject: Re: [PATCH v4 1/4] pwm: pca9685: Switch to atomic API
To:     Clemens Gruber <clemens.gruber@pqgruber.com>
Cc:     Thierry Reding <thierry.reding@gmail.com>,
        =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= 
        <u.kleine-koenig@pengutronix.de>, linux-pwm@vger.kernel.org,
        Lee Jones <lee.jones@linaro.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        David Jander <david@protonic.nl>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Dec 10, 2020 at 3:54 PM Clemens Gruber
<clemens.gruber@pqgruber.com> wrote:
>
> After reading your reasoning in this mail and rethinking the whole
> situation, I do no longer have any objections to the more complex
> solution. (Allowing 0% and 100% duty cycle channels with any period)
>
> I first thought it would be too confusing to block users from changing a
> duty cycle when in reality the period is the problem.
> However, if we log an error message, explaining that the periods have to
> match if duty cycles > 0 and < 100% are used, I think it's OK.
>
> Uwe, Sven: Do you have any objections?

No objections, as long as the "fully flexible" solution doesn't get too
complex. Complex code is hard to maintain and extend, but obviously that
decision is for Thierry or Uwe to make.

Thinking this through a little bit, I don't think the "fully flexible"
solution has to be that complex. Keeping track of prescale-inuse may
have to involve a bitmask and not a counter, such as here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pwm/pwm-pca9685.c?h=v5.10-rc7#n81

That way, channels can do set_bit() to mark that they use the prescaler,
clear_bit() when giving up the prescaler, and if (bitmap_weight() <= 1) to
check if they can make a prescale change.

But this is theory - the only way to find out what's best, is to actually
write the code...

> If not, I will prepare a v5 next week.

Looking forward to it, thank you !!