Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1221176 > unrolled thread

Re: [PATCH v5 0/6] bcm2835: auxiliar device support for spi

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v5 0/6] bcm2835: auxiliar device support for spi Eric Anholt <eric@anholt.net> - 2015-09-09 03:50 +0200
    Re: [PATCH v5 0/6] bcm2835: auxiliar device support for spi Alexander Stein <alexanders83@web.de> - 2015-09-09 11:10 +0200
      Re: [PATCH v5 0/6] bcm2835: auxiliar device support for spi Eric Anholt <eric@anholt.net> - 2015-09-09 20:30 +0200
    Re: [PATCH v5 0/6] bcm2835: auxiliar device support for spi Noralf Trønnes <noralf@tronnes.org> - 2015-09-10 17:50 +0200
      Re: [PATCH v5 0/6] bcm2835: auxiliar device support for spi Martin Sperl <kernel@martin.sperl.org> - 2015-09-10 18:00 +0200
        Re: [PATCH v5 0/6] bcm2835: auxiliar device support for spi Phil Elwell <phil@raspberrypi.org> - 2015-09-10 19:10 +0200

#1221176 — Re: [PATCH v5 0/6] bcm2835: auxiliar device support for spi

[Multipart message — attachments visible in raw view] — view raw

kernel@martin.sperl.org writes:

> From: Martin Sperl <kernel@martin.sperl.org>
>
> The BCM2835 contains 3 auxiliar devices:
> * spi1
> * spi2
> * uart1
>
> All of those 3 devices are enabled/disabled via a shared register,
> which is set by default to be disabled.
>
> Access to this register needs to get serialized.
>
> So after several iterations of discussions with the following ideas:
> * syscon - device tree should describe HW not drivers to use -
>            'compatiblity = "brcm,bcm2835-aux-enable", "syscon";'
>            is not acceptable
> * regulator - it is not necessarily a regulator or a power gate
>               that is implemented in HW, so it is not valid to use
>               this framework
>
> The recommendation was made to create a new minimal API in soc
> just for access to this shared enable/disable register.
>
> This patch-series implements:
> * the bcm2835-auxiliar device enable/disable api in soc.
> * the bcm2835-auxiliar spi device driver
>
> The uart1 device driver (ns16550 based) is not implemented so far
> but would be using the same API.
>
> Both spi and uart drivers can run with shared interrupts,
> so there is no need for an interrupt-controller to get implemented.

I finally had a chance to sit down and look at what the hardware's doing
with the enable bit (also, I've read a whole lot more of the hardware
now, so I'm a lot faster at answering questions like this).  The enable
bits are a clock gate off of the VPU clock.

I knocked together the enable bits as a clock gate driver, since I'd
just written very similar code for the audio domain clock driver (and I
assume you are grumpy about how much time you've spent on this one
stupid register).  It's up at
https://github.com/anholt/linux/tree/bcm2835-clock-aux and I can submit
it if you like the result.  I've compile tested it only, but I'm hoping
you could just drop your aux SPI driver on top of it and have things
work.

[toc] | [next] | [standalone]

#1221318

Hi,
On Tuesday 08 September 2015 18:48:07, Eric Anholt wrote:
> I finally had a chance to sit down and look at what the hardware's doing
> with the enable bit (also, I've read a whole lot more of the hardware
> now, so I'm a lot faster at answering questions like this).  The enable
> bits are a clock gate off of the VPU clock.

Are any hardware documents about such things available (in public)?

> I knocked together the enable bits as a clock gate driver, since I'd
> just written very similar code for the audio domain clock driver (and I
> assume you are grumpy about how much time you've spent on this one
> stupid register).  It's up at
> https://github.com/anholt/linux/tree/bcm2835-clock-aux and I can submit
> it if you like the result.  I've compile tested it only, but I'm hoping
> you could just drop your aux SPI driver on top of it and have things
> work.

IMHO line 45 (https://github.com/anholt/linux/commit/facb4ba917a1b9f6c2ee0cea7d529acf55f584dd#diff-1b6f753c132811b3f6d70f5b31866950R45) should be like this
> onecell->clks = kzalloc(sizeof(*onecell->clks) * BCM2835_AUX_CLOCK_COUNT, GFP_KERNEL);
or you will only allocate a single struct clk*.

Best regards,
Alexander

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1221613

[Multipart message — attachments visible in raw view] — view raw

Alexander Stein <alexanders83@web.de> writes:

> Hi,
> On Tuesday 08 September 2015 18:48:07, Eric Anholt wrote:
>> I finally had a chance to sit down and look at what the hardware's doing
>> with the enable bit (also, I've read a whole lot more of the hardware
>> now, so I'm a lot faster at answering questions like this).  The enable
>> bits are a clock gate off of the VPU clock.
>
> Are any hardware documents about such things available (in public)?

Nope, I just went through the HDL to see how things were routed.

>> I knocked together the enable bits as a clock gate driver, since I'd
>> just written very similar code for the audio domain clock driver (and I
>> assume you are grumpy about how much time you've spent on this one
>> stupid register).  It's up at
>> https://github.com/anholt/linux/tree/bcm2835-clock-aux and I can submit
>> it if you like the result.  I've compile tested it only, but I'm hoping
>> you could just drop your aux SPI driver on top of it and have things
>> work.
>
> IMHO line 45 (https://github.com/anholt/linux/commit/facb4ba917a1b9f6c2ee0cea7d529acf55f584dd#diff-1b6f753c132811b3f6d70f5b31866950R45) should be like this
>> onecell->clks = kzalloc(sizeof(*onecell->clks) * BCM2835_AUX_CLOCK_COUNT, GFP_KERNEL);
> or you will only allocate a single struct clk*.

Thanks, that was a bug in my other clock driver, too!

[toc] | [prev] | [next] | [standalone]

#1222270

Den 09.09.2015 03:48, skrev Eric Anholt:
> kernel@martin.sperl.org writes:
>
>> From: Martin Sperl <kernel@martin.sperl.org>
>>
>> The BCM2835 contains 3 auxiliar devices:
>> * spi1
>> * spi2
>> * uart1
>>
>> All of those 3 devices are enabled/disabled via a shared register,
>> which is set by default to be disabled.
>>
>> Access to this register needs to get serialized.
>>
>> So after several iterations of discussions with the following ideas:
>> * syscon - device tree should describe HW not drivers to use -
>>             'compatiblity = "brcm,bcm2835-aux-enable", "syscon";'
>>             is not acceptable
>> * regulator - it is not necessarily a regulator or a power gate
>>                that is implemented in HW, so it is not valid to use
>>                this framework
>>
>> The recommendation was made to create a new minimal API in soc
>> just for access to this shared enable/disable register.
>>
>> This patch-series implements:
>> * the bcm2835-auxiliar device enable/disable api in soc.
>> * the bcm2835-auxiliar spi device driver
>>
>> The uart1 device driver (ns16550 based) is not implemented so far
>> but would be using the same API.
>>
>> Both spi and uart drivers can run with shared interrupts,
>> so there is no need for an interrupt-controller to get implemented.
> I finally had a chance to sit down and look at what the hardware's doing
> with the enable bit (also, I've read a whole lot more of the hardware
> now, so I'm a lot faster at answering questions like this).  The enable
> bits are a clock gate off of the VPU clock.
>
> I knocked together the enable bits as a clock gate driver, since I'd
> just written very similar code for the audio domain clock driver (and I
> assume you are grumpy about how much time you've spent on this one
> stupid register).  It's up at
> https://github.com/anholt/linux/tree/bcm2835-clock-aux and I can submit
> it if you like the result.  I've compile tested it only, but I'm hoping
> you could just drop your aux SPI driver on top of it and have things
> work.
>

This looks interesting.
But there's a challenge with the uart1 and the 8250 driver.

Phil Elwell has this to say:
This means that that UART1 isn't an exact clone of a 8250 UART.
In a particular, the clock divisor is calculated differently.
A standard 8250 derives the baud rate as clock/(divisor16),
whereas the BCM2835 mini UART uses clock/(divisor8). This means
that if you want to use the standard driver then you need to lie
about the clock frequency, providing a value is twice the real
value, in order for a suitable divisor to be calculated.

Ref: https://github.com/raspberrypi/linux/pull/1008#issuecomment-139234607

So either we need a new uart1 driver or a doubled clock freq. somehow.


Noralf.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222271

> On 10.09.2015, at 17:48, Noralf Trønnes <noralf@tronnes.org> wrote:
> 
> This looks interesting.
> But there's a challenge with the uart1 and the 8250 driver.
> 
> Phil Elwell has this to say:
> This means that that UART1 isn't an exact clone of a 8250 UART.
> In a particular, the clock divisor is calculated differently.
> A standard 8250 derives the baud rate as clock/(divisor16),
> whereas the BCM2835 mini UART uses clock/(divisor8). This means
> that if you want to use the standard driver then you need to lie
> about the clock frequency, providing a value is twice the real
> value, in order for a suitable divisor to be calculated.
> 
> Ref: https://github.com/raspberrypi/linux/pull/1008#issuecomment-139234607
> 
> So either we need a new uart1 driver or a doubled clock freq. somehow.

Found out the same thing and communicated it to Eric - not 
knowing about the different divider…

Martin--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222314

[ Sending again in plain text ]
Noralf pointed me at fixed-factor-clock, and that works in our
(downstream) environment:

    soc: soc {
        ...
        uart1: uart@7e215040 {
            compatible = "brcm,bcm2835-aux-uart", "ns16550";
            reg = <0x7e215040 0x40>;
            interrupts = <1 29>;
            clocks = <&clk_uart1>;
            reg-shift = <2>;
            no-loopback-test;
            status = "disabled";
            };
    };

    clocks: clocks {
        ...
        clk_core: clock@2 {
            compatible = "fixed-clock";
            reg = <2>;
            #clock-cells = <0>;
            clock-output-names = "core";
            clock-frequency = <250000000>;
        };
        ...
        clk_uart1: clock@6 {
            compatible = "fixed-factor-clock";
            clocks = <&clk_core>;
            #clock-cells = <0>;
            clock-div = <1>;
            clock-mult = <2>;
        };
    };

Phil

On 10/09/2015 16:57, Martin Sperl wrote:
>> On 10.09.2015, at 17:48, Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>> This looks interesting.
>> But there's a challenge with the uart1 and the 8250 driver.
>>
>> Phil Elwell has this to say:
>> This means that that UART1 isn't an exact clone of a 8250 UART.
>> In a particular, the clock divisor is calculated differently.
>> A standard 8250 derives the baud rate as clock/(divisor16),
>> whereas the BCM2835 mini UART uses clock/(divisor8). This means
>> that if you want to use the standard driver then you need to lie
>> about the clock frequency, providing a value is twice the real
>> value, in order for a suitable divisor to be calculated.
>>
>> Ref: https://github.com/raspberrypi/linux/pull/1008#issuecomment-139234607
>>
>> So either we need a new uart1 driver or a doubled clock freq. somehow.
> Found out the same thing and communicated it to Eric - not 
> knowing about the different divider…
>
> Martin



On 10/09/2015 16:57, Martin Sperl wrote:
>> On 10.09.2015, at 17:48, Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>> This looks interesting.
>> But there's a challenge with the uart1 and the 8250 driver.
>>
>> Phil Elwell has this to say:
>> This means that that UART1 isn't an exact clone of a 8250 UART.
>> In a particular, the clock divisor is calculated differently.
>> A standard 8250 derives the baud rate as clock/(divisor16),
>> whereas the BCM2835 mini UART uses clock/(divisor8). This means
>> that if you want to use the standard driver then you need to lie
>> about the clock frequency, providing a value is twice the real
>> value, in order for a suitable divisor to be calculated.
>>
>> Ref: https://github.com/raspberrypi/linux/pull/1008#issuecomment-139234607
>>
>> So either we need a new uart1 driver or a doubled clock freq. somehow.
> Found out the same thing and communicated it to Eric - not 
> knowing about the different divider…
>
> Martin

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web