On 2017-11-17 12:26, Bjorn Andersson wrote:
On Thu 16 Nov 22:36 PST 2017, kgunda@xxxxxxxxxxxxxx wrote:
On 2017-11-16 22:25, Bjorn Andersson wrote:
> On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:
>
> > WLED driver provides the interface to the display driver to
> > adjust the brightness of the display backlight.
> >
>
> Hi Kiran,
>
> This driver has a lot in common with the already upstream pm8941-wled.c,
> because it's just a new revision of the same block.
>
> Please extend the existing driver rather than providing a new one
> (and yes, renaming the file is okay).
>
> Regards,
> Bjorn
Hi Bjorn,
Yes this driver design is similar to pm8941, however the WLED HW block
has undergone quite a few changes in analog and digital from PM8941 to
PM8998.
I can see that, looking at the documentation.
Few of them include splitting one module into wled-ctrl and wled-sink
peripherals, changes in the register offsets and the bit
interpretation.
This is typical and something we need to handle in all these drivers,
to
avoid having one driver per platform.
Hence we concluded that it was better to have a new driver to support
this new gen WELD module and decouple it from the pm8941.
Okay, I can see how it's easier to not have to case about anything but
pmi8998 in this driver, but where do you add the support for other WLED
versions? What about PMI8994? Will there not be similar differences
(registers that has moved around) in the future?
Also, going forward this driver will support AMOLED AVDD rail (not
supported by pm8941) touching a few more registers/configuration and
newer PMICs.
Is this a feature that was introduced in PMI8998? Will this support not
be dependent on the pmic version?
So spinning off a new driver would make it cleaner and easier to
extend further.
It's for sure easier at this point in time, but your argumentation
implies that PMI8998+1 should go into it's own driver as well.
I suspect that if you're going to reuse this driver for future PMIC
versions you will have to deal with register layout differences and new
feature set, and as such I'm not convinced that a new driver is needed.
Can you give any concrete examples of where it is not possible or
undesirable to maintain the pm8941 support in the same driver?
Regards,
Bjorn
Hi Bjorn,
Thanks for the inputs! Following are the reasons to go for the new
driver
and this driver can support 5 PMICs.
1.Majority of register, offsets and config values don’t match up
between
PMI8998 and PM8941
2.Feature such as – SC protection handling in SW cannot be done for 8941
as
there is no SC event/irq, AMOELD AVDD cannot supported by PM8941
3.Feature such as – string auto-calibration even if common will have to
use
different offsets/registers in the same SW logic
4.PMI8998, PMI8994, PMI8950 and PM660 all of them have this same WLED
module
(and register map) with very minor changes unlike 8941.
Thanks,
Kiran
--
To unsubscribe from this list: send the line "unsubscribe
linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
