Re: [PATCH v6 2/4] spi: bcm2835: add bcm2835 auxiliary spi device driver

2 messages, 2 authors, 2016-01-04 · open the first message on its own page

Re: [PATCH v6 2/4] spi: bcm2835: add bcm2835 auxiliary spi device driver

From: Stephan Olbrich <hidden>
Date: 2016-01-04 13:51:12 

Hi Martin,

I have tested the auxiliary spi device driver. It works in general for me, but 
I have two issues regarding the clock an chip select timing.

Am Freitag, 11. September 2015, 11:22:04 schrieb kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org:
quoted hunk
+static int bcm2835aux_spi_transfer_one(struct spi_master *master,
+				       struct spi_device *spi,
+				       struct spi_transfer *tfr)
+{
+	struct bcm2835aux_spi *bs = spi_master_get_devdata(master);
+	unsigned long spi_hz, clk_hz, speed;
+	unsigned long spi_used_hz, xfer_time_us;
+
+	/* calculate the registers to handle
+	 *
+	 * note that we use the variable data mode, which
+	 * is not optimal for longer transfers as we waste registers
+	 * resulting (potentially) in more interrupts when transferring
+	 * more than 12 bytes
+	 */
+	bs->cntl[0] = BCM2835_AUX_SPI_CNTL0_ENABLE |
+		      BCM2835_AUX_SPI_CNTL0_VAR_WIDTH |
+		      BCM2835_AUX_SPI_CNTL0_MSBF_OUT;
+	bs->cntl[1] = BCM2835_AUX_SPI_CNTL1_MSBF_IN;
+
+	/* set clock */
+	spi_hz = tfr->speed_hz;
+	clk_hz = clk_get_rate(bs->clk);
+
+	if (spi_hz >= clk_hz / 2) {
+		speed = 0;
+	} else if (spi_hz) {
+		speed = DIV_ROUND_UP(clk_hz, 2 * spi_hz) - 1;
+		if (speed >  BCM2835_AUX_SPI_CNTL0_SPEED_MAX)
+			speed = BCM2835_AUX_SPI_CNTL0_SPEED_MAX;
+	} else { /* the slowest we can go */
+		speed = BCM2835_AUX_SPI_CNTL0_SPEED_MAX;
+	}
+	bs->cntl[0] |= speed << BCM2835_AUX_SPI_CNTL0_SPEED_SHIFT;
+
+	spi_used_hz = clk_hz / (2 * (speed + 1));
+
+	/* handle all the modes */
+	if (spi->mode & SPI_CPOL)
+		bs->cntl[0] |= BCM2835_AUX_SPI_CNTL0_CPOL;

3 more quoted lines

According to the spi documentation [1]: "CPHA indicates the clock phase used 
to sample data; CPHA=0 says sample on the leading edge, CPHA=1 means the 
trailing edge."
Which is from my understanding different from what the BMC2835 ARM Peripherals 
[2] says for BCM2835_AUX_SPI_CNTL0_CPHA_IN:
"If 1 data is clocked in on the rising edge of the SPI clock
If 0 data is clocked in on the falling edge of the SPI clock"

I would expect the bits to be set dependant on the clock polarity (CPOL).

The other issue I have, is that the chip select is set before the clock 
polarity and the polarity is reset and set again between each transfers of a 
message. If CPOL is set to 1 this leads to additional rising and falling edges 
of the clock while chip select is active, where no data is sampled.

Stephan

[1] http://lxr.free-electrons.com/source/Documentation/spi/spi-summary
[2] https://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pdf



--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 2/4] spi: bcm2835: add bcm2835 auxiliary spi device driver

From: Martin Sperl <hidden>
Date: 2016-01-04 14:23:52 

On 04.01.2016, at 14:51, Stephan Olbrich [off-list ref] wrote:

According to the spi documentation [1]: "CPHA indicates the clock phase used 
to sample data; CPHA=0 says sample on the leading edge, CPHA=1 means the 
trailing edge."
Which is from my understanding different from what the BMC2835 ARM Peripherals 
[2] says for BCM2835_AUX_SPI_CNTL0_CPHA_IN:
"If 1 data is clocked in on the rising edge of the SPI clock
If 0 data is clocked in on the falling edge of the SPI clock"

I would expect the bits to be set dependant on the clock polarity (CPOL).
I am only having “typical” devices with do Mode 0,0 or 1,1 for which it
works.

Please come up with the “correct” logic for this to be set up correctly
and create the corresponding patch, as I do not have a corresponding
device for testing.

The other issue I have, is that the chip select is set before the clock 
polarity and the polarity is reset and set again between each transfers of a 
message. If CPOL is set to 1 this leads to additional rising and falling edges 
of the clock while chip select is active, where no data is sampled.
This is something I have seen before with the main spi HW.
We may need to port the same thing there.

The corresponding patch for spi-bcm2835.c is this:
acace73df2c1913a526c1b41e4741a4a6704c863

So we may need to move the corresponding logic in the same manner.

But this may be a more common problem when using GPIO-CS
which may need to get addressed at the spi-core level to avoid
such complications in other/future drivers - or it needs to be
“well documented” and may require a different “hook”, as the use
of prepare_message may not be 100% correct...

Martin


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help