Re: PinebookPro misc drivers..

On Sun, 21 Mar 2021 11:19:57 +0100
Emmanuel Vadot <[email protected]> wrote:

> On Sun, 21 Mar 2021 11:02:28 +0100
> Søren Schmidt <[email protected]> wrote:
> 
> > On 10 Mar 2021, at 16.01, Emmanuel Vadot <[email protected]> wrote:
> > > 
> > > On Wed, 10 Mar 2021 15:29:21 +0100
> > > Søren Schmidt <[email protected]> wrote:
> > > 
> > >> Hi
> > >> 
> > >> I?ve uploaded the latest from my PinebookPro collection here:
> > >> https://people.freebsd.org/~sos/PinebookPro/ <https://people.freebsd.org/~sos/PinebookPro/>
> > >> 
> > >> Comment,, bugs, etc welcome?
> > > 
> > > Few comments,
> > > 
> > > - Could you at least share patches generated with git diff ?
> > 
> > No git here, but added patches to the one file (rk_gpio.c) that isn?t new :)
> 
>  Why was all the softc variable renamed, this makes reviewing much
> harder than it should be.
> 
> > > - rk_gpiokeys.c doesn't seems correct, we already have
> > > sys/dev/gpio/gpiokeys.c so patch this one if it isn't enough for lid
> > > switch need.
> > 
> > Well, the stock drivers handling of this is, well, less than optimal and the lid support is pretty unique to the pbp so I decided to go this way as to not ?pollute? the original.
> 
>  Less than optimal why ?
>  Pretty unique why ?
>  I don't see anything unique for the pbp in the dts and if the driver
> that we already have is missing some stuff it should be patched.

 So, since I'm a nice guy I've looked at the driver.
 It is unique because you made it unique.
 Upon lid switch even if the lid closed you deactivate the lcd power
gpio control pin (which is not referenced in the dts node for the lid
switch) using a really a gross hack. We should never do that.
 You've also written that you haven't been able to get interrupts when
you open the lid but since you only configure it with
GPIO_INTR_EDGE_FALLING that's normal. What is happening if you
configure it with GPIO_INTR_EDGE_BOTH ? If it's still not working it
probably shows a problem in your gpio patch that add interrupts support.

> > > I could comment more if I would be able to diff more easily.
> > 
> > As stated above there is now a patch for you for rk_gpio.c the rest has nothing to diff against.
> > 
> > BTW I added support for writing an updated u-boot-2021.01 to the SPI flash on the pbp (also works for rockpro64), so one can boot from eMMC, SDcard, USB and NVMe with kbd/mouse support and screen output from the first u-boot output.
> 
>  And again no patches make this really hard to review.
>  Please create a phabricator account and post patches there, we can
> then have a proper discussion.
> 
> > BTW2: the pinebookpro dock from pine64 seems to have an issue, if you turn the USB-C connector ?wrong? you will lose alt the USB3 device in it. Simply turn the plug 180 degrees and everything works.
> > 
> > 
> > --
> > Søren Schmidt
> > [email protected] / [email protected]
> > "So much code to hack, so little time"
> > 
> > 
> > 
> 
> 
> -- 
> Emmanuel Vadot <[email protected]> <[email protected]>
> _______________________________________________
> [email protected] mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-arm
> To unsubscribe, send any mail to "[email protected]"


-- 
Emmanuel Vadot <[email protected]> <[email protected]>
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[email protected]"