Hello Simon,
On 02/20/2015 08:29 PM, Simon Glass wrote:
Hi,
On 20 February 2015 at 10:50, Stephen Warren <swar...@wwwdotorg.org> wrote:
On 02/20/2015 02:34 AM, Przemyslaw Marczak wrote:
Hello,
On 02/19/2015 06:09 PM, Stephen Warren wrote:
On 02/19/2015 05:11 AM, Przemyslaw Marczak wrote:
Hello,
On 02/18/2015 05:39 PM, Stephen Warren wrote:
On 02/17/2015 10:01 PM, Simon Glass wrote:
+Stephen who might have an opinion on this.
Hi Przemyslaw,
On 17 February 2015 at 06:09, Przemyslaw Marczak
<p.marc...@samsung.com> wrote:
This commits extends:
- dm gpio ops by: 'set_pull' call
- dm gpio uclass by: dm_gpio_set_pull() function
The pull mode is not defined so should be driver specific.
It's good to implement this, but I think you should try to have a
standard interface. You could define the options you want to support
and pass in a standard value.
Otherwise we are not really providing a driver abstraction, only an
interface.
I don't think that pull is a GPIO-related function/property. At
least on
Tegra, the GPIO controller allows you to set the pin direction and the
output value and that's it. Configuring pull-up/down and other IO
related properties is done in the pinmux controller instead. I don't
think we want a standard API that has to touch both HW modules at once.
What common code needs to manipulate a GPIO's pull-up/down setting? As
precedent observe that pull-up/down isn't part of the Linux kernel's
GPIO API, but rather that's part of the SoC-specific pinctrl driver,
which controls pinmuxing etc.
This is a quite different than in the Exynos, where all the gpio
functions and properties can be set by few registers within range of
each gpio port base address. So in this case we don't touch another
hardware module, we modify one of available gpio related registers.
Anyway, if we want to have a single and common gpio API in the future,
then I think it is better to add pull option.
Why? I'll ask again: What common driver code needs to manipulate
pull-ups?
Please look at driver: drivers/gpio/s5p_gpio.c
It's one driver related to one gpio hardware submodule and it takes care
about standard gpio properties and also mux/pull/drv/rate.
And the exynos pinmux code is only a software abstraction:
arch/arm/cpu/armv7/exynos/pinmux.c
I didn't want to ask which driver implements the control of pullups, but
rather which other driver needs to turn pullups on/off in a standard way
across multiple SoCs.
In other words, do you expect code in common/ to need to call a "set pin
pullup" function? If so, then we certainly need a standard API to manipulate
pullups. However if no common code needs to manipulate pullups, then I'd
argue we don't actually need a common API to do this, since there's no code
that would call that common API.
We do currently use the GPIO to handle pullup/pulldown for some boards
so until we have a pinmux API (which might be a long while) it seems
reasonable for it to live there.
If not, does anyone plan to write such an API?
Right, we uses this in most Exynos boards. But the boards uses direct
calls to s5p gpio driver, without uclass.
I wonder if wouldn't it be better and faster to leave the board
low-level init routines as they are now.
> And the driver will
implement what is required, instead of provide common and private api
for each driver.
I'm not proposing driver-specific APIs, but rather having a common GPIO
API and a common pinmux API. They need to be different since different
HW modules may implement the functionality.
As in the above example, for the Exynos it's the one hw module, so it's
simply.
For the various devices it is unclear, what should be pinmux and what
should be gpio driver.
How about the following are GPIO:
* Set GPIO pin direction
* Read GPIO input
* Set GPIO output value
... and anything else is pinmux. That's the split in Linux and AFAIK it
works out fine.
It'd be perfectly fine for the same driver code to implement both a GPIO
and a pinmux driver, if the HW supports it.
Ok, I can drop this commit, since the current code works fine.
Moreover in my opinion from the single external pin point of view the
pull up/down is the property of that pin.
It's a property of the same pin, but semantically it's not manipulating
a GPIO-related function.
Actually for Exynos, the pinmux is an abstraction and uses only GPIO
driver api in U-Boot.
Do we need pinmux class?
Best regards,
As I wrote in one of my previous e-mail, I was testing eMMC detect.
And setting the pull was required for this, before call the pinmux for
eMMC pins.
But finally the eMMC detect seem to be not useful in case of the present
'mmc rescan' command.
Why wouldn't the pinmux driver for the whole system simply apply the board's
whole pinmux configuration before initializing any IO controller drivers? IO
controller drivers shouldn't have to initialize board-/SoC-specific pinmux,
but the board-/SoC-specfic code should do so.
At most, the eMMC driver should call a function such as pinmux_emmc(), and
the board/SoC code should implement that as appropriate for that board. The
eMMC driver shouldn't have to know about applying specific pullups/downs to
specific pins (since those settings and pins are likely board-/SoC-specific,
and drivers shouldn't know about board-/SoC-specific details). The only
No this way lies madness. It is how things work on Jetson and Nyan.
Loads of opaque tables and no idea what the pins are connected to. It
has some value for pins that U-Boot doesn't use (so we are just
setting them up for Linux) but even then it is really opaque.
We can't even sent patches to the file because it is auto-generated
from a tool in another repo. Tiny differences between boards are
hidden because we repeat all the information again with just a line or
two of changes. I really don't want exynos to go that way.
exception would be if the standard IO protocol requires pullups to be
changed during regular operation. In which case, a specific callback from
the driver could be added for each protocol-mandated configuration change,
thus keeping the IO controller driver still completely isolated from details
of the pins and pinmux APIs etc.
This is like the 'funcmux' in Tegra I think. I think this is more
useful and we should use it to set up all peripheral pins. We can
review the code, see changes, understand what they relate to, etc.
Anyway this all seems off-topic from this patch.
Unless someone plans to write a pinmux subsystem for U-Boot (which I
agree would be better) I think the general approach of this patch is
good.
Regards,
Simon
Ok, so there are two next versions of this patch-set.
Please decide, which one is better.
For me, at present, the current s5p_gpio api works fine for all the
exynos based boards.
Introducing the pinmux uclass is not a quick task, now I'm trying to
focus on pmic.
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
