[RFC 1/4] realtek: rtl8231 driver: dts option for GPIO state

[Evan Jobling]

Firstly, thanks for the feedback!

> Are we sure that commit shouldn't be reverted?  Why is it be better to
> set all pins to some arbitrary state than leaving them in whatever state
> the bootloader (or we, before reboot) set them to?

It's a great point that another option was reverting that commit.

If someone could elaborate on more context for that patch it would be great.
I didn't want to break existing devices.

I guess another valid option is make that patch opt out in device tree?
(Or opt in, and see which targets are affected/who complains/when something
breaks?)
I think this issue is more how to handle existing configurations.
An option is update the device compat for all targets?

I think ensuring we aren't going to get unexpected behaviour is reasonable.

If output drivers don't work we can't trust what's on the rtl8231 GPIO's?
That's also the SFP's and other I2C devices I think?
I think it's mostly an issue for things that aren't bi-directional though?

i.e. what happens if we try to toggle a GPIO and we think the GPIO
output drivers are working and they aren't?
Fan example: we think we're controlling the fans but we aren't
i.e. only have open loop control, at least in the JG922A example.

I guess another option is just reset and ensure output driver
state on just the pins we care about?
 
> In addition to your fan problem, I'm thinking about the PoE enable pin
> on the Zyxel GS1900-10HP (and possibly other devices).  If we had a boot
> loader that didn't touch this output pin, then it would be nice to be
> able to load the driver without toggling it too.  Don't think your patch
> will be sufficient.  It will still toggle the pins briefly, won't it?

You are absolutely correct that this still toggles the pins.
When I was testing my larger, by register method I was so slow that
the fans "blipped". At the moment the fans don't blip for JG922A
with this by pin implementation. But because this method is by pin
and looping it will cause issues even with fan speed if you
sequenced the array in a particular fashion.

I can see the impact on a device that is rebooting but perhaps
you want the PoE to still be enabled through a reboot.

I had a (working.....) implementation that did lots of bitwise
math and getting masks to do it by register.

I was at the stage where I was doing to do my own review.
Didn't want to review my extra code before requesting feedback.
I lost interest as my code "worked" haha.
Plus then there's just another path that I wasn't planning on using.
My justification is that using the existing per pin functions
reduces the amount of code/bitwise arithmetic added that is just
another workaround.

One could extend the code to do it by register and not touch specific
things if they're in a "good" state later.

My point regarding "good" state is below though:

If the output driver isn't working whilst it's in output mode,
then how do you know when reading from the rtl8231? 
i.e. if it's in output mode the output driver is functioning?

Once you don't trust your initial state it's just that,
you need a "known good" state to work from.

i.e. My question is how do you determine the rtl8231 GPIO 
output driver state is "good"?
You would need to toggle the GPIO
and see what happens anyway?

Cheers,
Evan.