Hi Alex, On 12 August 2016 at 12:38, Alexander Graf <ag...@suse.de> wrote: > > > On 12.08.16 19:21, Simon Glass wrote: >> Hi Alex, >> >> On 11 August 2016 at 23:27, Alexander Graf <ag...@suse.de> wrote: >>> >>> >>>> Am 12.08.2016 um 00:38 schrieb Simon Glass <s...@chromium.org>: >>>> >>>> Hi Alex, >>>> >>>>> On 11 August 2016 at 05:33, Alexander Graf <ag...@suse.de> wrote: >>>>> >>>>> >>>>>> On 09.08.16 06:28, Stephen Warren wrote: >>>>>>> On 08/04/2016 05:15 PM, Alexander Graf wrote: >>>>>>> >>>>>>>> On 04 Aug 2016, at 20:11, Stephen Warren <swar...@wwwdotorg.org> wrote: >>>>>>>> >>>>>>>> On 08/04/2016 01:11 AM, Alexander Graf wrote: >>>>>>>>> On the raspberry pi, you can disable the serial port to gain dynamic >>>>>>>>> frequency >>>>>>>>> scaling which can get handy at times. >>>>>>>>> >>>>>>>>> However, in such a configuration the serial controller gets its rx >>>>>>>>> queue filled >>>>>>>>> up with zero bytes which then happily get transmitted on to whoever >>>>>>>>> calls >>>>>>>>> getc() today. >>>>>>>>> >>>>>>>>> This patch adds detection logic for that case by checking whether >>>>>>>>> the RX pin is >>>>>>>>> mapped to GPIO15 and disables the mini uart if it is not mapped >>>>>>>>> properly. >>>>>>>>> >>>>>>>>> That way we can leave the driver enabled in the tree and can >>>>>>>>> determine during >>>>>>>>> runtime whether serial is usable or not, having a single binary that >>>>>>>>> allows for >>>>>>>>> uart and non-uart operation. >>>>>>>> >>>>>>>>> diff --git a/drivers/serial/serial_bcm283x_mu.c >>>>>>>>> b/drivers/serial/serial_bcm283x_mu.c >>>>>>>> >>>>>>>>> @@ -72,9 +87,18 @@ static int bcm283x_mu_serial_probe(struct udevice >>>>>>>>> *dev) >>>>>>>>> { >>>>>>>>> struct bcm283x_mu_serial_platdata *plat = dev_get_platdata(dev); >>>>>>>>> struct bcm283x_mu_priv *priv = dev_get_priv(dev); >>>>>>>>> + struct bcm283x_gpio_regs *gpio = (struct bcm283x_gpio_regs >>>>>>>>> *)plat->gpio; >>>>>>>>> >>>>>>>>> priv->regs = (struct bcm283x_mu_regs *)plat->base; >>>>>>>>> >>>>>>>>> + /* >>>>>>>>> + * The RPi3 disables the mini uart by default. The easiest way >>>>>>>>> to find >>>>>>>>> + * out whether it is available is to check if the pin is muxed. >>>>>>>>> + */ >>>>>>>>> + if (((readl(&gpio->gpfsel1) >> BCM283X_GPIO_GPFSEL1_F15_SHIFT) & >>>>>>>>> + BCM283X_GPIO_ALTFUNC_MASK) != BCM283X_GPIO_ALTFUNC_5) >>>>>>>>> + priv->disabled = true; >>>>>>>>> + >>>>>>>>> return 0; >>>>>>>> >>>>>>>> Comment on the current implementation: Can't probe() return an error >>>>>>>> if the device should be disabled? That would avoid the need to check >>>>>>>> priv->disabled in all the other functions. >>>>>>> >>>>>>> I guess I should’ve put that in a comment somewhere. Unfortunately we >>>>>>> can’t. If I just return an error on probe, U-Boot will panic because >>>>>>> we tell it in a CONFIG define that we require a serial port (grep for >>>>>>> CONFIG_REQUIRE_SERIAL_CONSOLE). >>>>>>> >>>>>>> We could maybe try to unset that define instead? >>>>>> >>>>>> Yes, assuming that U-Boot runs just fine with HDMI console only, I think >>>>>> it's fine to unset CONFIG_REQUIRE_SERIAL_CONSOLE. >>>>>> >>>>>>>> Overall comment: I'd rather not put this logic into the UART driver >>>>>>>> itself; it is system-specific rather than device-specific. I'd also >>>>>>>> rather not have the UART driver touching GPIO registers; that's not >>>>>>>> very modular, and could cause problems if the Pi is converted to use >>>>>>>> DT to instantiate devices. >>>>>>>> >>>>>>>> Instead, can we put the logic into board/raspberrypi/rpi/rpi.c? I.e. >>>>>>>> have some early function come along and enable/disable the >>>>>>>> bcm2837_serials device object as appropriate? That way it isolates >>>>>>>> the code to the Pi specifically, and not any other bcm283x board. >>>>>>>> We'd want to wrap that code in #ifdef CONFIG_PL01X_SERIAL. >>>>>>> >>>>>>> We can do that if we can fail at probe time. If we absolutely must >>>>>>> have a serial driver to work in the first place, that doesn’t work. I >>>>>>> can try to poke at it, but it’ll be a few days I think :). >>>>> >>>>> So I couldn't find a sane way to fail probing based on something defined >>>>> in the board file, reusing the existing gpio device. >>>> >>>> Would it be possible to move this code into the serial driver? >>> >>> You mean like in v2 which Stephen nacked? :) >> >> Yes :-( >> >> Well you can put what you like in the board code, and if this is only >> on the rpi, then it makes sense. >> >> Really though, this is a pinctrl thing, so if there were a pinctrl >> driver you could just use it. The GPIO driver should not deal with pin >> muxing. > > It's the same IP block on the RPi :). > >> >>> >>>> >>>>> >>>>> However, there's an easy alternative. We can make the console code just >>>>> ignore our serial device if we set its pointer to NULL. That way we >>>>> still have the device, but can contain all logic to disable usage of the >>>>> mini uart to the board file. >>>> >>>> I'm not very keen on that - feels like a hack. What is stopping >>>> Stephen's idea from working? I could perhaps help with dm plumbing is >>>> that is the issue... >>> >>> The problem is that we need the gpio device to determine whether the pin is >>> muxed. There is no temporal control that I could see that would allow me to >>> be in a place where the gpio device exists, the serial device does not >>> exist, and where I could then not spawn the serial device based on board >>> logic. >> >> Can you use board_early_init_f() ? > > How? I guess we would need to > > a) Create the GPIO device > b) Ask the GPIO device whether the pin is muxed correctly > c) Create serial device based on outcome of b > > Is that possible?
Well you were asking for a place where you could check a GPIO before the serial driver is started. In board_early_init_f() you can check the GPIO and basically do what you are doing now. The only question is how to enable/disable the serial driver. One option is to add some platform data to the driver which has a 'disable' flag. Check that in the serial driver and return -EPERM from the probe() method. I think the plumbing will work from there, but have not tried it. As an example of this sort of hack, see board_init() in gurnard.c (it is used in atmel_fb_ofdata_to_platdata()). Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot