* [PATCH 56/62] watchdog: tangox_wdt: Convert to use device managed functions
From: Guenter Roeck @ 2017-01-11 14:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a45ed205-7c1b-a298-50c0-fc8b80a846c5@sigmadesigns.com>
On 01/11/2017 04:31 AM, Marc Gonzalez wrote:
> On 11/01/2017 11:52, Guenter Roeck wrote:
>
>> On 01/11/2017 01:07 AM, Marc Gonzalez wrote:
>>
>>>> @@ -134,12 +134,15 @@ static int tangox_wdt_probe(struct platform_device *pdev)
>>>> err = clk_prepare_enable(dev->clk);
>>>> if (err)
>>>> return err;
>>>> + err = devm_add_action_or_reset(&pdev->dev,
>>>> + (void(*)(void *))clk_disable_unprepare,
>>>> + dev->clk);
>>>> + if (err)
>>>> + return err;
>>>
>>> Hello Guenter,
>>>
>>> I would rather avoid the function pointer cast.
>>> How about defining an auxiliary function for the cleanup action?
>>>
>>> clk_disable_unprepare() is static inline, so gcc will have to
>>> define an auxiliary function either way. What do you think?
>>
>> Not really. It would just make it more complicated to replace the
>> call with devm_clk_prepare_enable(), should it ever find its way
>> into the light of day.
>
> More complicated, because the cleanup function will have to be deleted later?
> The compiler will warn if someone forgets to do that.
>
> In my opinion, it's not a good idea to rely on the fact that casting
> void(*)(struct clk *clk) to void(*)(void *) is likely to work as expected
> on most platforms. (It has undefined behavior, strictly speaking.)
>
I do hear that you object to this code.
However, I must admit that you completely lost me here. It is a cast from
one function pointer to another, passed as argument to another function,
with a secondary cast of its argument from a typed pointer to a void pointer.
I don't think C permits for "undefined behavior, strictly speaking".
Besides, that same mechanism is already used elsewhere, which is how I
got the idea. Are you claiming that there are situations where it won't
work ?
> Do you really dislike the portable solution I suggested? :-(
>
It is not more portable than the above. It is more expensive and adds more
code.
Thanks,
Guenter
^ permalink raw reply
* [PATCH 0/3] arm64: dts: A64 board MMC support
From: Andre Przywara @ 2017-01-11 14:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170111135139.ktirdwmzhyv3cn2q@lukather>
Hi,
On 11/01/17 13:51, Maxime Ripard wrote:
> Hi,
>
> On Tue, Jan 10, 2017 at 01:22:30AM +0000, Andre Przywara wrote:
>> These patches here go on top of Maxime's latest A64 MMC series and
>> enable the MMC controllers on the boards using the A64 SoC.
>> As the BananaPi-M64 DT now looks different, lets adds support for
>> that board as well, with one major difference to the Pine64 being the eMMC
>> chip.
>>
>> I could't find commit f9ca9b952ee1 Maxime mentioned in his cover letter,
>> so I applied his patches on top of sunxi/for-next and cherry-picked
>> b4b8664d29 to fix the arm64 build.
>
> I'm fine with all your patches. I've queued them in my MMC series, and
> will continue pushing them and / or merging them if it's ok for you.
Yes, please do!
Many thanks!
Cheers,
Andre.
^ permalink raw reply
* [PATCH] arm64: Add support for DMA_ATTR_SKIP_CPU_SYNC attribute to swiotlb
From: Robin Murphy @ 2017-01-11 14:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484129477-24121-1-git-send-email-geert+renesas@glider.be>
On 11/01/17 10:11, Geert Uytterhoeven wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>
> This patch adds support for DMA_ATTR_SKIP_CPU_SYNC attribute for
> dma_{un}map_{page,sg} functions family to swiotlb.
>
> DMA_ATTR_SKIP_CPU_SYNC allows platform code to skip synchronization of
> the CPU cache for the given buffer assuming that it has been already
> transferred to 'device' domain.
>
> Ported from IOMMU .{un}map_{sg,page} ops.
>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
> - Add Acked-by.
>
> Support for DMA_ATTR_SKIP_CPU_SYNC was included when porting the IOMMU
> ops from arm to arm64 in commit 13b8629f651164d7 ("arm64: Add IOMMU
> dma_ops").
>
> Presumably it was an oversight that the existing swiotlb based
> implementation didn't have support for DMA_ATTR_SKIP_CPU_SYNC yet?
Less an oversight, more that nobody's wanted to use it until now ;)
Personally I'd prefer flag tests to be "!(x)" rather than "(x) == 0",
but the latter is already in place, so I'll leave the final word on
style/consistency nitpicks to Catalin and Will.
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> ---
> arch/arm64/mm/dma-mapping.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index e04082700bb16c35..1d7d5d2881db7c19 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -211,7 +211,8 @@ static dma_addr_t __swiotlb_map_page(struct device *dev, struct page *page,
> dma_addr_t dev_addr;
>
> dev_addr = swiotlb_map_page(dev, page, offset, size, dir, attrs);
> - if (!is_device_dma_coherent(dev))
> + if (!is_device_dma_coherent(dev) &&
> + (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
> __dma_map_area(phys_to_virt(dma_to_phys(dev, dev_addr)), size, dir);
>
> return dev_addr;
> @@ -222,7 +223,8 @@ static void __swiotlb_unmap_page(struct device *dev, dma_addr_t dev_addr,
> size_t size, enum dma_data_direction dir,
> unsigned long attrs)
> {
> - if (!is_device_dma_coherent(dev))
> + if (!is_device_dma_coherent(dev) &&
> + (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
> __dma_unmap_area(phys_to_virt(dma_to_phys(dev, dev_addr)), size, dir);
> swiotlb_unmap_page(dev, dev_addr, size, dir, attrs);
> }
> @@ -235,7 +237,8 @@ static int __swiotlb_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
> int i, ret;
>
> ret = swiotlb_map_sg_attrs(dev, sgl, nelems, dir, attrs);
> - if (!is_device_dma_coherent(dev))
> + if (!is_device_dma_coherent(dev) &&
> + (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
> for_each_sg(sgl, sg, ret, i)
> __dma_map_area(phys_to_virt(dma_to_phys(dev, sg->dma_address)),
> sg->length, dir);
> @@ -251,7 +254,8 @@ static void __swiotlb_unmap_sg_attrs(struct device *dev,
> struct scatterlist *sg;
> int i;
>
> - if (!is_device_dma_coherent(dev))
> + if (!is_device_dma_coherent(dev) &&
> + (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
> for_each_sg(sgl, sg, nelems, i)
> __dma_unmap_area(phys_to_virt(dma_to_phys(dev, sg->dma_address)),
> sg->length, dir);
>
^ permalink raw reply
* [PATCH v8 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2017-01-11 14:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOAejn2eOy2sn1VkE979ne23Sj9L6+kaQDNpL1EUKb2m=6sGXw@mail.gmail.com>
>
>> + */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
>
> You could get rid of this, when caching the value of CR1. Would save two
> register reads here. This doesn't work for all registers, but it should
> be possible to apply for most of them, maybe enough to get rid of the
> clr_bits and set_bits function.
I agree at many places I could save registers read by not using
clr_bits and set_bits function when the registers in question has been
already read.
But it is not enough to get rid of the clr_bits and set_bits function.
For example when calling stm32f4_i2c_terminate_xfer(), the CR1
register is never read before so set_bits function is useful.
Another example, when stm32f4_i2c_handle_rx_done(), the CR1 register
is also never read before so clr_bits finction is again useful.
2017-01-11 14:58 GMT+01:00 M'boumba Cedric Madianga <cedric.madianga@gmail.com>:
> Hi Uwe,
>
> 2017-01-11 9:22 GMT+01:00 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
>> Hello Cedric,
>>
>> On Thu, Jan 05, 2017 at 10:07:23AM +0100, M'boumba Cedric Madianga wrote:
>>> +/*
>>> + * In standard mode:
>>> + * SCL period = SCL high period = SCL low period = CCR * I2C parent clk period
>>> + *
>>> + * In fast mode:
>>> + * If Duty = 0; SCL high period = 1 * CCR * I2C parent clk period
>>> + * SCL low period = 2 * CCR * I2C parent clk period
>>> + * If Duty = 1; SCL high period = 9 * CCR * I2C parent clk period
>>> + * SCL low period = 16 * CCR * I2C parent clk period
>> s/ \*/ */ several times
>
> Sorry but I don't see where is the issue as the style for multi-line
> comments seems ok.
> Could you please clarify that point if possible ? Thanks in advance
>
>>
>>> + * In order to reach 400 kHz with lower I2C parent clk frequencies we always set
>>> + * Duty = 1
>>> + *
>>> + * For both modes, we have CCR = SCL period * I2C parent clk frequency
>>> + * with scl_period = 5 microseconds in Standard mode and scl_period = 1
>> s/mode/Mode/
>
> ok thanks
>
>>
>>> + * microsecond in Fast Mode in order to satisfy scl_high and scl_low periods
>>> + * constraints defined by i2c bus specification
>>
>> I don't understand scl_period = 1 ?s for Fast Mode. For a bus freqency
>> of 400 kHz we need low + high = 2.5 ?s. Is there a factor 10 missing
>> somewhere?
>
> As CCR = SCL_period * I2C parent clk frequency with minimal freq =
> 2Mhz and SCL_period = 1 we have:
> CCR = 1 * 2Mhz = 2.
> But to compute, scl_low and scl_high in Fast mode, we have to do the
> following thing as Duty=1:
> scl_high = 9 * CCR * I2C parent clk period
> scl_low = 16 * CCR * I2C parent clk period
> In our example:
> scl_high = 9 * 2 * 0,0000005 = 0,000009 sec = 9 ?s
> scl_low = 16 * 2 * 0.0000005 = 0,000016 sec = 16 ?s
> So low + high = 27 ?s > 2,5 ?s
>
>>
>>> + */
>>> +static struct stm32f4_i2c_timings i2c_timings[] = {
>>> [...]
>>> +
>>> +/**
>>> + * stm32f4_i2c_hw_config() - Prepare I2C block
>>> + * @i2c_dev: Controller's private data
>>> + */
>>> +static int stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> + int ret = 0;
>>> +
>>> + /* Disable I2C */
>>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
>>> +
>>> + ret = stm32f4_i2c_set_periph_clk_freq(i2c_dev);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + stm32f4_i2c_set_rise_time(i2c_dev);
>>> +
>>> + stm32f4_i2c_set_speed_mode(i2c_dev);
>>> +
>>> + stm32f4_i2c_set_filter(i2c_dev);
>>> +
>>> + /* Enable I2C */
>>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_PE);
>>
>> This function is called after a hw reset, so there should be no need to
>> use clr_bits and set_bits because the value read from hw should be
>> known.
>
> ok thanks
>
>>
>>> + return ret;
>>
>> return 0;
>
> ok thanks
>
>>
>>> +}
>>> +
>>> +static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> + u32 status;
>>> + int ret;
>>> +
>>> + ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
>>> + status,
>>> + !(status & STM32F4_I2C_SR2_BUSY),
>>> + 10, 1000);
>>> + if (ret) {
>>> + dev_dbg(i2c_dev->dev, "bus not free\n");
>>> + ret = -EBUSY;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_write_ byte() - Write a byte in the data register
>>> + * @i2c_dev: Controller's private data
>>> + * @byte: Data to write in the register
>>> + */
>>> +static void stm32f4_i2c_write_byte(struct stm32f4_i2c_dev *i2c_dev, u8 byte)
>>> +{
>>> + writel_relaxed(byte, i2c_dev->base + STM32F4_I2C_DR);
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_write_msg() - Fill the data register in write mode
>>> + * @i2c_dev: Controller's private data
>>> + *
>>> + * This function fills the data register with I2C transfer buffer
>>> + */
>>> +static void stm32f4_i2c_write_msg(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> +
>>> + stm32f4_i2c_write_byte(i2c_dev, *msg->buf++);
>>> + msg->count--;
>>> +}
>>> +
>>> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> + u32 rbuf;
>>> +
>>> + rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
>>> + *msg->buf++ = rbuf & 0xff;
>>
>> This is unnecessary. buf has an 8 bit wide type so
>>
>> *msg->buf++ = rbuf;
>>
>> has the same effect. (ISTR this is something I already pointed out
>> earlier?)
>
> Yes you are right.
>
>>
>>> + msg->count--;
>>> +}
>>> +
>>> +static void stm32f4_i2c_terminate_xfer(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>>> +
>>> + stm32f4_i2c_disable_irq(i2c_dev);
>>> +
>>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> + if (msg->stop)
>>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>>> + else
>>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>>> +
>>> + complete(&i2c_dev->complete);
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_handle_write() - Handle FIFO empty interrupt in case of write
>>> + * @i2c_dev: Controller's private data
>>> + */
>>> +static void stm32f4_i2c_handle_write(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>>> +
>>> + if (msg->count) {
>>> + stm32f4_i2c_write_msg(i2c_dev);
>>> + if (!msg->count) {
>>> + /* Disable buffer interrupts for RXNE/TXE events */
>>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>>> + }
>>> + } else {
>>> + stm32f4_i2c_terminate_xfer(i2c_dev);
>>
>> Is stm32f4_i2c_terminate_xfer also called when arbitration is lost? If
>> yes, is it then right to set STM32F4_I2C_CR1_STOP or
>> STM32F4_I2C_CR1_START?
>
> If arbitration is lost, stm32f4_i2c_terminate_xfer() is not called.
> In that case, we return -EAGAIN and i2c-core will retry by calling
> stm32f4_i2c_xfer()
>
>>
>>> + }
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
>>> + * @i2c_dev: Controller's private data
>>> + */
>>> +static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>>> +
>>> + switch (msg->count) {
>>> + case 1:
>>> + stm32f4_i2c_disable_irq(i2c_dev);
>>> + stm32f4_i2c_read_msg(i2c_dev);
>>> + complete(&i2c_dev->complete);
>>> + break;
>>> + /*
>>> + * For 2 or 3-byte reception, we do not have to read the data register
>>> + * when RXNE occurs as we have to wait for byte transferred finished
>>
>> it's hard to understand because if you don't know the hardware the
>> meaning of RXNE is unknown.
>
> Ok I will replace RXNE by RX not empty in that comment
>
>>
>>> + * event before reading data. So, here we just disable buffer
>>> + * interrupt in order to avoid another system preemption due to RXNE
>>> + * event
>>> + */
>>> + case 2:
>>> + case 3:
>>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>>> + break;
>>> + /* For N byte reception with N > 3 we directly read data register */
>>> + default:
>>> + stm32f4_i2c_read_msg(i2c_dev);
>>> + }
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
>>> + * in case of read
>>> + * @i2c_dev: Controller's private data
>>> + */
>>> +static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>
>> btf is a hw-related name. Maybe better use _done which is easier to
>> understand?
>
> OK
>
>>
>>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> + void __iomem *reg;
>>> + u32 mask;
>>> + int i;
>>> +
>>> + switch (msg->count) {
>>> + case 2:
>>> + /*
>>> + * In order to correctly send the Stop or Repeated Start
>>> + * condition on the I2C bus, the STOP/START bit has to be set
>>> + * before reading the last two bytes.
>>> + * After that, we could read the last two bytes, disable
>>> + * remaining interrupts and notify the end of xfer to the
>>> + * client
>>
>> This is surprising. I didn't recheck the manual, but that looks very
>> uncomfortable.
>
> I agree but this exactly the hardware way of working described in the
> reference manual.
>
>>How does this work, when I only want to read a single
>> byte? Same problem for ACK below.
>
> For a single reception, we enable NACK and STOP or Repeatead START
> bits during address match.
> The NACK and STOP/START pulses are sent as soon as the data is
> received in the shift register.
> Please note that in that case, we don't have to wait BTF event to read the data.
> Data is read as soon as RXNE event occurs.
>
>>
>>> + */
>>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> + if (msg->stop)
>>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>>> + else
>>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>>> +
>>> + for (i = 2; i > 0; i--)
>>> + stm32f4_i2c_read_msg(i2c_dev);
>>> +
>>> + reg = i2c_dev->base + STM32F4_I2C_CR2;
>>> + mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
>>> + stm32f4_i2c_clr_bits(reg, mask);
>>> +
>>> + complete(&i2c_dev->complete);
>>> + break;
>>> + case 3:
>>> + /*
>>> + * In order to correctly send the ACK on the I2C bus for the
>>> + * last two bytes, we have to set ACK bit before reading the
>>> + * third last data byte
>>> + */
>>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>>> + stm32f4_i2c_read_msg(i2c_dev);
>>> + break;
>>> + default:
>>> + stm32f4_i2c_read_msg(i2c_dev);
>>> + }
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
>>> + * master receiver
>>> + * @i2c_dev: Controller's private data
>>> + */
>>> +static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> + void __iomem *reg;
>>> +
>>> + switch (msg->count) {
>>> + case 0:
>>> + stm32f4_i2c_terminate_xfer(i2c_dev);
>>> + /* Clear ADDR flag */
>>> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>>> + break;
>>> + case 1:
>>> + /*
>>> + * Single byte reception:
>>
>> This also happens for the last byte of a 5 byte transfer, right?
>
> For a 5 byte transfer the behavior is different:
> We have to read data from DR (data register) as soon as the RXNE (RX
> not empty) event occurs for data1, data2 and data3 (until N-2 data for
> a more generic case)
> The ACK is automatically sent as soon as the data is received in the
> shift register as the I2C controller was configured to do that during
> adress match phase.
>
> For data3 (N-2 data), we wait for BTF (Byte Transfer finished) event
> in order to set NACK before reading DR.
> This event occurs when a new data has been received in shift register
> (in our case data4 or N-1 data) but the prevoius data in DR (in our
> case data3 or N-2 data) has not been read yet.
> In that way, the NACK pulse will be correctly generated after the last
> received data byte.
>
> For data4 and data5, we wait for BTF event (data4 or N-1 data in DR
> and data5 or N data in shift register), set STOP or repeated Start in
> order to correctly sent the right pulse after the last received data
> byte and run 2 consecutives read of DR.
>
>>
>>> + * Enable NACK, clear ADDR flag and generate STOP or RepSTART
>>> + */
>>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>>> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>>> + if (msg->stop)
>>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>>> + else
>>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>>> + break;
>>> + case 2:
>>> + /*
>>> + * 2-byte reception:
>>> + * Enable NACK and set POS
>>
>> What is POS?
> POS is used to define the position of the (N)ACK pulse
> 0: ACK is generated when the current is being received in the shift register
> 1: ACK is generated when the next byte which will be received in the
> shift register (used for 2-byte reception)
>
>>
>>> + */
>>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
>>
>> You could get rid of this, when caching the value of CR1. Would save two
>> register reads here. This doesn't work for all registers, but it should
>> be possible to apply for most of them, maybe enough to get rid of the
>> clr_bits and set_bits function.
>>
>>> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>>> + break;
>>> +
>>> + default:
>>> + /* N-byte reception: Enable ACK */
>>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
>>
>> Do you need to set ACK for each byte transferred?
> I need to do that in order to be SMBus compatible and the ACK/NACK
> seems to be used by default in Documentation/i2c/i2c-protocol file.
>
>>
>> I stopp reviewing here because of -ENOTIME on my side but don't want to
>> delay discussion, so sent my comments up to here already now.
>>
>> Best regards
>> Uwe
>>
>> --
>> Pengutronix e.K. | Uwe Kleine-K?nig |
>> Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH v6 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order
From: Hanjun Guo @ 2017-01-11 14:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ef759a3a-bbdb-2586-24a8-6ad650b07d41@suse.com>
On 01/11/2017 06:20 PM, Matthias Brugger wrote:
>
>
> On 02/01/17 14:31, Hanjun Guo wrote:
>> The head file is strictly in alphabetic order now, so let's
>> be the rule breaker. As acpi_iort.h includes acpi.h so remove
>> the duplidate acpi.h inclusion as well.
>>
>
> Sounds strange, maybe someting like:
> Rearrange header file includes to alphabetic order. As acpi_iort.h...
It's better, will update the patch.
Thanks
Hanjun
^ permalink raw reply
* [PATCH v6 05/14] ACPI: platform-msi: retrieve dev id from IORT
From: Hanjun Guo @ 2017-01-11 14:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110145741.GA17967@red-moon>
On 01/10/2017 10:57 PM, Lorenzo Pieralisi wrote:
> On Tue, Jan 10, 2017 at 09:39:39PM +0800, Hanjun Guo wrote:
>
> [...]
>
>>> What you can do is create a wrapper, say iort_node_map_platform_id()
>>> (whose signature is equivalent to iort_node_map_rid() minus rid_in)
>>> that carries out the two steps outlined above.
>>>
>>> To do that I suggest the following:
>>>
>>> (1) I send a patch to "fix" iort_node_get_id() (ie index issue you
>>> reported)
>>
>> I prepared two simple patches, one is for fix the indentation and
>> the other is adding the missing kernel-doc comment, how about
>> sending the out for 4.10-rcx?
>
> For me it is fine depending on how Rafael wants to handle them,
> ie if he can batch those with the eg iort_node_get_id() fix I have
> just sent:
>
> https://patchwork.kernel.org/patch/9507041/
>
>>> (2) We remove type_mask handling from iort_node_get_id()
>>
>> iort_node_get_id() for now only supports id single mappings,
>> Do we need to extend it for multi id mappings? seems Sinan's
>> platform have such cases.
>
> I am not really sure I understand what you mean here.
Sorry for not clear, I was thinking if we want to support
ID mapping entries with multi IDs like BDFs for RC,
>
>>> (3) We create iort_node_map_platform_id() that (pseudo-code, I can
>>> write the patch if it is clearer):
>>>
>>> struct acpi_iort_node *iort_node_map_platform_id(u8 type_mask, int index,
>>> ...)
>>> {
>>> u32 id, id_out;
>>> struct acpi_iort_node *parent = iort_node_get_id(&id, index);
>>>
>>> if (!parent)
>>> return NULL;
>>>
>>> /* we should probably rename iort_node_map_rid() too */
>>> if (!(IORT_TYPE_MASK(parent->type) & type_mask)
>>> parent = iort_node_map_rid(parent, id, &id_out, type_mask);
>>>
>>> return parent;
>>> }
>>>
>>> (4) we update current iort_node_get_id() users and move them over
>>> to iort_node_map_platform_id()
>>
>> I think we need to prepare one patch for the above steps, or it
>> have functional changes for iort_node_get_id(), for example we
>> removed the type_mask handling from iort_node_get_id() and it
>> will break the case for SMMU if we only have requester id entries.
>
> If the question is "should we apply this change as a single logical
> patch" the answer is yes, it looks a simple one to me (basically
> it implies writing the function above and update the iort_node_get_id()
> existing callers with it). Does this answer your question ?
Yes, thank you for your patience :)
When I was preparing patches, I split them into three patches, hope it
makes the review easier, will send out the patch set soon.
Thanks
Hanjun
^ permalink raw reply
* [RFC PATCH] IOMMU: SMMUv2: Support for Extended Stream ID (16 bit)
From: Robin Murphy @ 2017-01-11 14:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110115755.19102-1-aleksey.makarov@linaro.org>
On 10/01/17 11:57, Aleksey Makarov wrote:
> Enable the Extended Stream ID feature when available.
>
> This patch on top of series "[PATCH v7 00/19] KVM PCIe/MSI passthrough
> on ARM/ARM64 and IOVA reserved regions" by Eric Auger allows
> to passthrough an external PCIe network card on a ThunderX server
> successfully.
>
> Without this patch that card caused a warning like
>
> pci 0006:90:00.0: stream ID 0x9000 out of range for SMMU (0x7fff)
>
> during boot.
>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
> drivers/iommu/arm-smmu.c | 53 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 37 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 13d26009b8e0..d160c12828f4 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -24,6 +24,7 @@
> * - v7/v8 long-descriptor format
> * - Non-secure access to the SMMU
> * - Context fault reporting
> + * - Extended Stream ID (16 bit)
> */
>
> #define pr_fmt(fmt) "arm-smmu: " fmt
> @@ -87,6 +88,7 @@
> #define sCR0_CLIENTPD (1 << 0)
> #define sCR0_GFRE (1 << 1)
> #define sCR0_GFIE (1 << 2)
> +#define sCR0_EXIDENABLE (1 << 3)
> #define sCR0_GCFGFRE (1 << 4)
> #define sCR0_GCFGFIE (1 << 5)
> #define sCR0_USFCFG (1 << 10)
> @@ -126,6 +128,7 @@
> #define ID0_NUMIRPT_MASK 0xff
> #define ID0_NUMSIDB_SHIFT 9
> #define ID0_NUMSIDB_MASK 0xf
> +#define ID0_EXIDS (1 << 8)
> #define ID0_NUMSMRG_SHIFT 0
> #define ID0_NUMSMRG_MASK 0xff
>
> @@ -169,6 +172,7 @@
> #define ARM_SMMU_GR0_S2CR(n) (0xc00 + ((n) << 2))
> #define S2CR_CBNDX_SHIFT 0
> #define S2CR_CBNDX_MASK 0xff
> +#define S2CR_EXIDVALID (1 << 10)
> #define S2CR_TYPE_SHIFT 16
> #define S2CR_TYPE_MASK 0x3
> enum arm_smmu_s2cr_type {
> @@ -354,6 +358,7 @@ struct arm_smmu_device {
> #define ARM_SMMU_FEAT_FMT_AARCH64_64K (1 << 9)
> #define ARM_SMMU_FEAT_FMT_AARCH32_L (1 << 10)
> #define ARM_SMMU_FEAT_FMT_AARCH32_S (1 << 11)
> +#define ARM_SMMU_FEAT_EXIDS (1 << 12)
> u32 features;
>
> #define ARM_SMMU_OPT_SECURE_CFG_ACCESS (1 << 0)
> @@ -1051,7 +1056,7 @@ static void arm_smmu_write_smr(struct arm_smmu_device *smmu, int idx)
> struct arm_smmu_smr *smr = smmu->smrs + idx;
> u32 reg = smr->id << SMR_ID_SHIFT | smr->mask << SMR_MASK_SHIFT;
>
> - if (smr->valid)
> + if (!(smmu->features & ARM_SMMU_FEAT_EXIDS) && smr->valid)
> reg |= SMR_VALID;
> writel_relaxed(reg, ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_SMR(idx));
> }
> @@ -1063,6 +1068,9 @@ static void arm_smmu_write_s2cr(struct arm_smmu_device *smmu, int idx)
> (s2cr->cbndx & S2CR_CBNDX_MASK) << S2CR_CBNDX_SHIFT |
> (s2cr->privcfg & S2CR_PRIVCFG_MASK) << S2CR_PRIVCFG_SHIFT;
>
> + if (smmu->features & ARM_SMMU_FEAT_EXIDS && smmu->smrs &&
> + smmu->smrs[idx].valid)
> + reg |= S2CR_EXIDVALID;
> writel_relaxed(reg, ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_S2CR(idx));
> }
>
> @@ -1674,6 +1682,9 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
> if (smmu->features & ARM_SMMU_FEAT_VMID16)
> reg |= sCR0_VMID16EN;
>
> + if (smmu->features & ARM_SMMU_FEAT_EXIDS)
> + reg |= sCR0_EXIDENABLE;
> +
> /* Push the button */
> __arm_smmu_tlb_sync(smmu);
> writel(reg, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
> @@ -1761,7 +1772,12 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
> "\t(IDR0.CTTW overridden by FW configuration)\n");
>
> /* Max. number of entries we have for stream matching/indexing */
> - size = 1 << ((id >> ID0_NUMSIDB_SHIFT) & ID0_NUMSIDB_MASK);
> + if (smmu->version == ARM_SMMU_V2 && id & ID0_EXIDS) {
> + smmu->features |= ARM_SMMU_FEAT_EXIDS;
> + size = (1 << 16);
Unnecessary parentheses.
> + } else {
> + size = 1 << ((id >> ID0_NUMSIDB_SHIFT) & ID0_NUMSIDB_MASK);
> + }
Given what the architecture says about the relationship between EXIDS
and NUMSIDB, I suppose an even shorter version could be:
if (smmu->version == ARM_SMMU_V2 && id & ID0_EXIDS)
size *= 2;
but I'm not sure that's actually any nicer to read.
> smmu->streamid_mask = size - 1;
> if (id & ID0_SMS) {
> u32 smr;
> @@ -1774,20 +1790,25 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
> return -ENODEV;
> }
>
> - /*
> - * SMR.ID bits may not be preserved if the corresponding MASK
> - * bits are set, so check each one separately. We can reject
> - * masters later if they try to claim IDs outside these masks.
> - */
> - smr = smmu->streamid_mask << SMR_ID_SHIFT;
> - writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
> - smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
> - smmu->streamid_mask = smr >> SMR_ID_SHIFT;
> -
> - smr = smmu->streamid_mask << SMR_MASK_SHIFT;
> - writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
> - smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
> - smmu->smr_mask_mask = smr >> SMR_MASK_SHIFT;
> + if (smmu->features & ARM_SMMU_FEAT_EXIDS) {
> + smmu->smr_mask_mask = smmu->streamid_mask;
> + } else {
> + /*
> + * SMR.ID bits may not be preserved if the corresponding
> + * MASK bits are set, so check each one separately.
> + * We can reject masters later if they try to claim IDs
> + * outside these masks.
> + */
> + smr = smmu->streamid_mask << SMR_ID_SHIFT;
> + writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
> + smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
> + smmu->streamid_mask = smr >> SMR_ID_SHIFT;
> +
> + smr = smmu->streamid_mask << SMR_MASK_SHIFT;
> + writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
> + smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
> + smmu->smr_mask_mask = smr >> SMR_MASK_SHIFT;
> + }
This hunk is quite possibly wrong. I don't see any guarantee in the
architecture that all EXMASK/EXID bits *must* be implemented, and even
so there's still no harm in the driver determining that experimentally.
It looks like we need a bit of refactoring such that we move the probing
of SMR fields to after counting and allocating the SME structures, then
in the EXIDS case we can explicitly clear the SMEs and poke EXIDENABLE
inbetween.
Robin.
>
> /* Zero-initialised to mark as invalid */
> smmu->smrs = devm_kcalloc(smmu->dev, size, sizeof(*smmu->smrs),
>
^ permalink raw reply
* [PATCH] ARM: defconfig: qcom: add APQ8060 DragonBoard devices
From: Neil Armstrong @ 2017-01-11 14:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7762461.Mcpxx0AiPM@wuerfel>
On 01/11/2017 03:08 PM, Arnd Bergmann wrote:
> On Wednesday, January 11, 2017 2:19:55 PM CET Linus Walleij wrote:
>> On Tue, Jan 10, 2017 at 3:53 PM, Andy Gross <andy.gross@linaro.org> wrote:
>>> On Tue, Jan 10, 2017 at 10:55:21AM +0100, Linus Walleij wrote:
>>>> This default-enables the devices found on the APQ8060 DragonBoard
>>>> in the qcom_defconfig:
>>>>
>>>> - EBI2 bus
>>>> - SMSC911x ethernet
>>>> - LEDs class and PM8058 LEDs driver, trigger and heartbeat
>>>> trigger (so we get heartbeat on the board by default)
>>>> - IIO framework, including the HRTimer trigger, KXSD9
>>>> accelerometer, MPU3050 gyroscope, AK8975 magnetometer and
>>>> BMP085 pressure sensor
>>>>
>>>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> This brings up a point of discussion. Do we even need the qcom_defconfig any
>>> more? Is everyone comfortable with using the multi_v7_defconfig?
>
> I think having one specialized defconfig for the platform is helpful for
> the build/boot testing, e.g. it can show whether a boot failure with
> multi_v7_defconfig is the result of a qcom-specific change, or a side-effect
> of something that was done on another platform.
>
>>> Aside from size of the image, i can't think of any other reason to keep around
>>> the separate qcom file.
>>
>> Actually a bit of Arnd/Olof question.
>>
>> Bystander opinion below:
>>
>> That is pretty much up to the maintainer (you) I guess.
>> Reasons would be:
>>
>> - Lower footprint (because you may not need all stuff selected
>> as 'y' compiled-in in multi_v7) on some platforms this is even
>> necessary to get a bootable image or one that will load in
>> reasonable time.
>>
>> - Enable a few things by default (both compiled-in and modules)
>> that multi_v7 would consider to be littering
>>
>> - For "my" systems I usually like them because these defconfigs
>> have vastly shorter compile time (because so much stuff that
>> idon't concern me is left out).
>>
>> On the other hand: some ARMv7 system maintainers have x86
>> ambitions: compile once, run everywhere, and certainly that is
>> the ambition with multi_v7, and if that overshadows all the above,
>> just kill off qcom_defconfig and be happy :)
>
> We recently killed of the Broadcom defconfig file that actually
> contained some very different platforms that had not much in common
> besides the company name.
>
> I think my preference is to keep it, but if Andy wants it removed
> and nobody complains, that's fine too.
>
> Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Hi all,
In fact, as far as I remember the multi_v7 did not fit on the MDM9615
due to it's limited memory available to Linux.
Neil
^ permalink raw reply
* [PATCH] ARM: defconfig: qcom: add APQ8060 DragonBoard devices
From: Arnd Bergmann @ 2017-01-11 14:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdb+bH1bj14d-2rBMadJQLCdVgdeuc07ESkuQpWt9UQ6rg@mail.gmail.com>
On Wednesday, January 11, 2017 2:19:55 PM CET Linus Walleij wrote:
> On Tue, Jan 10, 2017 at 3:53 PM, Andy Gross <andy.gross@linaro.org> wrote:
> > On Tue, Jan 10, 2017 at 10:55:21AM +0100, Linus Walleij wrote:
> >> This default-enables the devices found on the APQ8060 DragonBoard
> >> in the qcom_defconfig:
> >>
> >> - EBI2 bus
> >> - SMSC911x ethernet
> >> - LEDs class and PM8058 LEDs driver, trigger and heartbeat
> >> trigger (so we get heartbeat on the board by default)
> >> - IIO framework, including the HRTimer trigger, KXSD9
> >> accelerometer, MPU3050 gyroscope, AK8975 magnetometer and
> >> BMP085 pressure sensor
> >>
> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >
> > This brings up a point of discussion. Do we even need the qcom_defconfig any
> > more? Is everyone comfortable with using the multi_v7_defconfig?
I think having one specialized defconfig for the platform is helpful for
the build/boot testing, e.g. it can show whether a boot failure with
multi_v7_defconfig is the result of a qcom-specific change, or a side-effect
of something that was done on another platform.
> > Aside from size of the image, i can't think of any other reason to keep around
> > the separate qcom file.
>
> Actually a bit of Arnd/Olof question.
>
> Bystander opinion below:
>
> That is pretty much up to the maintainer (you) I guess.
> Reasons would be:
>
> - Lower footprint (because you may not need all stuff selected
> as 'y' compiled-in in multi_v7) on some platforms this is even
> necessary to get a bootable image or one that will load in
> reasonable time.
>
> - Enable a few things by default (both compiled-in and modules)
> that multi_v7 would consider to be littering
>
> - For "my" systems I usually like them because these defconfigs
> have vastly shorter compile time (because so much stuff that
> idon't concern me is left out).
>
> On the other hand: some ARMv7 system maintainers have x86
> ambitions: compile once, run everywhere, and certainly that is
> the ambition with multi_v7, and if that overshadows all the above,
> just kill off qcom_defconfig and be happy :)
We recently killed of the Broadcom defconfig file that actually
contained some very different platforms that had not much in common
besides the company name.
I think my preference is to keep it, but if Andy wants it removed
and nobody complains, that's fine too.
Arnd
^ permalink raw reply
* [PATCH] coresight: STM: Balance enable/disable
From: Suzuki K Poulose @ 2017-01-11 13:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAG2=9p8CMZD37KPbp+LCyDeizUHaDmw0NfqQHPvzKdCR9L0XZw@mail.gmail.com>
On 11/01/17 11:41, Chunyan Zhang wrote:
> On 11 January 2017 at 01:36, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>> On Tue, Jan 10, 2017 at 11:21:55AM +0000, Suzuki K Poulose wrote:
>>> The stm is automatically enabled when an application sets the policy
>>> via ->link() call back by using coresight_enable(), which keeps the
>>> refcount of the current users of the STM. However, the unlink() callback
>>> issues stm_disable() directly, which leaves the STM turned off, without
>>> the coresight layer knowing about it. This prevents any further uses
>>> of the STM hardware as the coresight layer still thinks the STM is
>>> turned on and doesn't issue an stm_enable(). Even manually enabling
>>> the STM via sysfs can't really enable the hw.
>>>
>>> e.g,
>>>
>>> $ echo 1 > $CS_DEVS/$ETR/enable_sink
>>> $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
>>> $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
>>> $ echo 64 > $CS_DEVS/$source/traceid
>>> $ ./stm_app
>>> Sending 64000 byte blocks of pattern 0 at 0us intervals
>>> Success to map channel(32768~32783) to 0xffffa95fa000
>>> Sending on channel 32768
>>> $ dd if=/dev/$ETR of=~/trace.bin.1
>>> 597+1 records in
>>> 597+1 records out
>>> 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
>>> $ ./stm_app
>>> Sending 64000 byte blocks of pattern 0 at 0us intervals
>>> Success to map channel(32768~32783) to 0xffff7e9e2000
>>> Sending on channel 32768
>>> $ dd if=/dev/$ETR of=~/trace.bin.2
>>> 0+0 records in
>>> 0+0 records out
>>> 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s
>>>
>>> Note that we don't get any data from the ETR for the second session.
>>>
>>> Also dmesg shows :
>>>
>>> [ 77.520458] coresight-tmc 20800000.etr: TMC-ETR enabled
>>> [ 77.537097] coresight-replicator etr_replicator at 20890000: REPLICATOR enabled
>>> [ 77.558828] coresight-replicator main_replicator at 208a0000: REPLICATOR enabled
>>> [ 77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
>>> [ 77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
>>> [ 77.618422] coresight-stm 20860000.stm: STM tracing enabled
>>> [ 139.554252] coresight-stm 20860000.stm: STM tracing disabled
>>> # End of first tracing session
>>> [ 146.351135] coresight-tmc 20800000.etr: TMC read start
>>> [ 146.514486] coresight-tmc 20800000.etr: TMC read end
>>> # Note that the STM is not turned on via stm_generic_link()->coresight_enable()
>>> # and hence none of the components are turned on.
>>> [ 152.479080] coresight-tmc 20800000.etr: TMC read start
>>> [ 152.542632] coresight-tmc 20800000.etr: TMC read end
>>>
>>> This patch balances the unlink operation by using the coresight_disable(),
>>> keeping the coresight layer in sync with the hardware state.
>>>
>>> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM component")
>>> Cc: Pratik Patel <pratikp@codeaurora.org>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: stable at vger.kernel.org # 4.7+
>>> Reported-by: Robert Walker <robert.walker@arm.com>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>> drivers/hwtracing/coresight/coresight-stm.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
>>> index 3524452..57b7330 100644
>>> --- a/drivers/hwtracing/coresight/coresight-stm.c
>>> +++ b/drivers/hwtracing/coresight/coresight-stm.c
>>> @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
>>> if (!drvdata || !drvdata->csdev)
>>> return;
>>>
>>> - stm_disable(drvdata->csdev, NULL);
>>> + coresight_disable(drvdata->csdev);
>>
>> This looks valid to me.
>>
>> Chunyan, any reason to use stm_disable() directly rather than calling it as part
>> of the device OPS in coresight_disable()?
>
> I don't think there's some special reason for this. I simply hadn't
> noticed that these two operations didn't use two balanced functions.
Please can I have an Ack/Reviewed -by on it, so that we can push it
as a fix.
Suzuki
^ permalink raw reply
* [PATCH v8 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2017-01-11 13:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170111082208.vzu7xgpd4eakyldl@pengutronix.de>
Hi Uwe,
2017-01-11 9:22 GMT+01:00 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> Hello Cedric,
>
> On Thu, Jan 05, 2017 at 10:07:23AM +0100, M'boumba Cedric Madianga wrote:
>> +/*
>> + * In standard mode:
>> + * SCL period = SCL high period = SCL low period = CCR * I2C parent clk period
>> + *
>> + * In fast mode:
>> + * If Duty = 0; SCL high period = 1 * CCR * I2C parent clk period
>> + * SCL low period = 2 * CCR * I2C parent clk period
>> + * If Duty = 1; SCL high period = 9 * CCR * I2C parent clk period
>> + * SCL low period = 16 * CCR * I2C parent clk period
> s/ \*/ */ several times
Sorry but I don't see where is the issue as the style for multi-line
comments seems ok.
Could you please clarify that point if possible ? Thanks in advance
>
>> + * In order to reach 400 kHz with lower I2C parent clk frequencies we always set
>> + * Duty = 1
>> + *
>> + * For both modes, we have CCR = SCL period * I2C parent clk frequency
>> + * with scl_period = 5 microseconds in Standard mode and scl_period = 1
> s/mode/Mode/
ok thanks
>
>> + * microsecond in Fast Mode in order to satisfy scl_high and scl_low periods
>> + * constraints defined by i2c bus specification
>
> I don't understand scl_period = 1 ?s for Fast Mode. For a bus freqency
> of 400 kHz we need low + high = 2.5 ?s. Is there a factor 10 missing
> somewhere?
As CCR = SCL_period * I2C parent clk frequency with minimal freq =
2Mhz and SCL_period = 1 we have:
CCR = 1 * 2Mhz = 2.
But to compute, scl_low and scl_high in Fast mode, we have to do the
following thing as Duty=1:
scl_high = 9 * CCR * I2C parent clk period
scl_low = 16 * CCR * I2C parent clk period
In our example:
scl_high = 9 * 2 * 0,0000005 = 0,000009 sec = 9 ?s
scl_low = 16 * 2 * 0.0000005 = 0,000016 sec = 16 ?s
So low + high = 27 ?s > 2,5 ?s
>
>> + */
>> +static struct stm32f4_i2c_timings i2c_timings[] = {
>> [...]
>> +
>> +/**
>> + * stm32f4_i2c_hw_config() - Prepare I2C block
>> + * @i2c_dev: Controller's private data
>> + */
>> +static int stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + int ret = 0;
>> +
>> + /* Disable I2C */
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
>> +
>> + ret = stm32f4_i2c_set_periph_clk_freq(i2c_dev);
>> + if (ret)
>> + return ret;
>> +
>> + stm32f4_i2c_set_rise_time(i2c_dev);
>> +
>> + stm32f4_i2c_set_speed_mode(i2c_dev);
>> +
>> + stm32f4_i2c_set_filter(i2c_dev);
>> +
>> + /* Enable I2C */
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_PE);
>
> This function is called after a hw reset, so there should be no need to
> use clr_bits and set_bits because the value read from hw should be
> known.
ok thanks
>
>> + return ret;
>
> return 0;
ok thanks
>
>> +}
>> +
>> +static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + u32 status;
>> + int ret;
>> +
>> + ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
>> + status,
>> + !(status & STM32F4_I2C_SR2_BUSY),
>> + 10, 1000);
>> + if (ret) {
>> + dev_dbg(i2c_dev->dev, "bus not free\n");
>> + ret = -EBUSY;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_write_ byte() - Write a byte in the data register
>> + * @i2c_dev: Controller's private data
>> + * @byte: Data to write in the register
>> + */
>> +static void stm32f4_i2c_write_byte(struct stm32f4_i2c_dev *i2c_dev, u8 byte)
>> +{
>> + writel_relaxed(byte, i2c_dev->base + STM32F4_I2C_DR);
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_write_msg() - Fill the data register in write mode
>> + * @i2c_dev: Controller's private data
>> + *
>> + * This function fills the data register with I2C transfer buffer
>> + */
>> +static void stm32f4_i2c_write_msg(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> +
>> + stm32f4_i2c_write_byte(i2c_dev, *msg->buf++);
>> + msg->count--;
>> +}
>> +
>> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> + u32 rbuf;
>> +
>> + rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
>> + *msg->buf++ = rbuf & 0xff;
>
> This is unnecessary. buf has an 8 bit wide type so
>
> *msg->buf++ = rbuf;
>
> has the same effect. (ISTR this is something I already pointed out
> earlier?)
Yes you are right.
>
>> + msg->count--;
>> +}
>> +
>> +static void stm32f4_i2c_terminate_xfer(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> + stm32f4_i2c_disable_irq(i2c_dev);
>> +
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + if (msg->stop)
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> + else
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +
>> + complete(&i2c_dev->complete);
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_write() - Handle FIFO empty interrupt in case of write
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_write(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> + if (msg->count) {
>> + stm32f4_i2c_write_msg(i2c_dev);
>> + if (!msg->count) {
>> + /* Disable buffer interrupts for RXNE/TXE events */
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> + }
>> + } else {
>> + stm32f4_i2c_terminate_xfer(i2c_dev);
>
> Is stm32f4_i2c_terminate_xfer also called when arbitration is lost? If
> yes, is it then right to set STM32F4_I2C_CR1_STOP or
> STM32F4_I2C_CR1_START?
If arbitration is lost, stm32f4_i2c_terminate_xfer() is not called.
In that case, we return -EAGAIN and i2c-core will retry by calling
stm32f4_i2c_xfer()
>
>> + }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> + switch (msg->count) {
>> + case 1:
>> + stm32f4_i2c_disable_irq(i2c_dev);
>> + stm32f4_i2c_read_msg(i2c_dev);
>> + complete(&i2c_dev->complete);
>> + break;
>> + /*
>> + * For 2 or 3-byte reception, we do not have to read the data register
>> + * when RXNE occurs as we have to wait for byte transferred finished
>
> it's hard to understand because if you don't know the hardware the
> meaning of RXNE is unknown.
Ok I will replace RXNE by RX not empty in that comment
>
>> + * event before reading data. So, here we just disable buffer
>> + * interrupt in order to avoid another system preemption due to RXNE
>> + * event
>> + */
>> + case 2:
>> + case 3:
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> + break;
>> + /* For N byte reception with N > 3 we directly read data register */
>> + default:
>> + stm32f4_i2c_read_msg(i2c_dev);
>> + }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
>> + * in case of read
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>
> btf is a hw-related name. Maybe better use _done which is easier to
> understand?
OK
>
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> + void __iomem *reg;
>> + u32 mask;
>> + int i;
>> +
>> + switch (msg->count) {
>> + case 2:
>> + /*
>> + * In order to correctly send the Stop or Repeated Start
>> + * condition on the I2C bus, the STOP/START bit has to be set
>> + * before reading the last two bytes.
>> + * After that, we could read the last two bytes, disable
>> + * remaining interrupts and notify the end of xfer to the
>> + * client
>
> This is surprising. I didn't recheck the manual, but that looks very
> uncomfortable.
I agree but this exactly the hardware way of working described in the
reference manual.
>How does this work, when I only want to read a single
> byte? Same problem for ACK below.
For a single reception, we enable NACK and STOP or Repeatead START
bits during address match.
The NACK and STOP/START pulses are sent as soon as the data is
received in the shift register.
Please note that in that case, we don't have to wait BTF event to read the data.
Data is read as soon as RXNE event occurs.
>
>> + */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + if (msg->stop)
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> + else
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +
>> + for (i = 2; i > 0; i--)
>> + stm32f4_i2c_read_msg(i2c_dev);
>> +
>> + reg = i2c_dev->base + STM32F4_I2C_CR2;
>> + mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
>> + stm32f4_i2c_clr_bits(reg, mask);
>> +
>> + complete(&i2c_dev->complete);
>> + break;
>> + case 3:
>> + /*
>> + * In order to correctly send the ACK on the I2C bus for the
>> + * last two bytes, we have to set ACK bit before reading the
>> + * third last data byte
>> + */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> + stm32f4_i2c_read_msg(i2c_dev);
>> + break;
>> + default:
>> + stm32f4_i2c_read_msg(i2c_dev);
>> + }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
>> + * master receiver
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>> + void __iomem *reg;
>> +
>> + switch (msg->count) {
>> + case 0:
>> + stm32f4_i2c_terminate_xfer(i2c_dev);
>> + /* Clear ADDR flag */
>> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> + break;
>> + case 1:
>> + /*
>> + * Single byte reception:
>
> This also happens for the last byte of a 5 byte transfer, right?
For a 5 byte transfer the behavior is different:
We have to read data from DR (data register) as soon as the RXNE (RX
not empty) event occurs for data1, data2 and data3 (until N-2 data for
a more generic case)
The ACK is automatically sent as soon as the data is received in the
shift register as the I2C controller was configured to do that during
adress match phase.
For data3 (N-2 data), we wait for BTF (Byte Transfer finished) event
in order to set NACK before reading DR.
This event occurs when a new data has been received in shift register
(in our case data4 or N-1 data) but the prevoius data in DR (in our
case data3 or N-2 data) has not been read yet.
In that way, the NACK pulse will be correctly generated after the last
received data byte.
For data4 and data5, we wait for BTF event (data4 or N-1 data in DR
and data5 or N data in shift register), set STOP or repeated Start in
order to correctly sent the right pulse after the last received data
byte and run 2 consecutives read of DR.
>
>> + * Enable NACK, clear ADDR flag and generate STOP or RepSTART
>> + */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> + if (msg->stop)
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> + else
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> + break;
>> + case 2:
>> + /*
>> + * 2-byte reception:
>> + * Enable NACK and set POS
>
> What is POS?
POS is used to define the position of the (N)ACK pulse
0: ACK is generated when the current is being received in the shift register
1: ACK is generated when the next byte which will be received in the
shift register (used for 2-byte reception)
>
>> + */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
>
> You could get rid of this, when caching the value of CR1. Would save two
> register reads here. This doesn't work for all registers, but it should
> be possible to apply for most of them, maybe enough to get rid of the
> clr_bits and set_bits function.
>
>> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> + break;
>> +
>> + default:
>> + /* N-byte reception: Enable ACK */
>> + reg = i2c_dev->base + STM32F4_I2C_CR1;
>> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
>
> Do you need to set ACK for each byte transferred?
I need to do that in order to be SMBus compatible and the ACK/NACK
seems to be used by default in Documentation/i2c/i2c-protocol file.
>
> I stopp reviewing here because of -ENOTIME on my side but don't want to
> delay discussion, so sent my comments up to here already now.
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-K?nig |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH 2/2] crypto: mediatek - fix format string for 64-bit builds
From: Arnd Bergmann @ 2017-01-11 13:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170111135104.3961730-1-arnd@arndb.de>
After I enabled COMPILE_TEST for non-ARM targets, I ran into these
warnings:
crypto/mediatek/mtk-aes.c: In function 'mtk_aes_info_map':
crypto/mediatek/mtk-aes.c:224:28: error: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Werror=format=]
dev_err(cryp->dev, "dma %d bytes error\n", sizeof(*info));
crypto/mediatek/mtk-sha.c:344:28: error: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Werror=format=]
crypto/mediatek/mtk-sha.c:550:21: error: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'size_t {aka long unsigned int}' [-Werror=format=]
The correct format for size_t is %zu, so use that in all three
cases.
Fixes: 785e5c616c84 ("crypto: mediatek - Add crypto driver support for some MediaTek chips")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/crypto/mediatek/mtk-aes.c | 2 +-
drivers/crypto/mediatek/mtk-sha.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/mediatek/mtk-aes.c b/drivers/crypto/mediatek/mtk-aes.c
index 3271471060d9..1370cabeeb5b 100644
--- a/drivers/crypto/mediatek/mtk-aes.c
+++ b/drivers/crypto/mediatek/mtk-aes.c
@@ -221,7 +221,7 @@ static int mtk_aes_info_map(struct mtk_cryp *cryp,
aes->ct_dma = dma_map_single(cryp->dev, info, sizeof(*info),
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(cryp->dev, aes->ct_dma))) {
- dev_err(cryp->dev, "dma %d bytes error\n", sizeof(*info));
+ dev_err(cryp->dev, "dma %zu bytes error\n", sizeof(*info));
return -EINVAL;
}
aes->tfm_dma = aes->ct_dma + sizeof(*ct);
diff --git a/drivers/crypto/mediatek/mtk-sha.c b/drivers/crypto/mediatek/mtk-sha.c
index 89513632c8ed..98b3d74ae23d 100644
--- a/drivers/crypto/mediatek/mtk-sha.c
+++ b/drivers/crypto/mediatek/mtk-sha.c
@@ -341,7 +341,7 @@ static int mtk_sha_info_map(struct mtk_cryp *cryp,
sha->ct_dma = dma_map_single(cryp->dev, info, sizeof(*info),
DMA_BIDIRECTIONAL);
if (unlikely(dma_mapping_error(cryp->dev, sha->ct_dma))) {
- dev_err(cryp->dev, "dma %d bytes error\n", sizeof(*info));
+ dev_err(cryp->dev, "dma %zu bytes error\n", sizeof(*info));
return -EINVAL;
}
sha->tfm_dma = sha->ct_dma + sizeof(*ct);
@@ -547,7 +547,7 @@ static int mtk_sha_update_slow(struct mtk_cryp *cryp,
final = (ctx->flags & SHA_FLAGS_FINUP) && !ctx->total;
- dev_dbg(cryp->dev, "slow: bufcnt: %u\n", ctx->bufcnt);
+ dev_dbg(cryp->dev, "slow: bufcnt: %zu\n", ctx->bufcnt);
if (final) {
sha->flags |= SHA_FLAGS_FINAL;
--
2.9.0
^ permalink raw reply related
* [PATCH 0/3] arm64: dts: A64 board MMC support
From: Maxime Ripard @ 2017-01-11 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484011353-21480-1-git-send-email-andre.przywara@arm.com>
Hi,
On Tue, Jan 10, 2017 at 01:22:30AM +0000, Andre Przywara wrote:
> These patches here go on top of Maxime's latest A64 MMC series and
> enable the MMC controllers on the boards using the A64 SoC.
> As the BananaPi-M64 DT now looks different, lets adds support for
> that board as well, with one major difference to the Pine64 being the eMMC
> chip.
>
> I could't find commit f9ca9b952ee1 Maxime mentioned in his cover letter,
> so I applied his patches on top of sunxi/for-next and cherry-picked
> b4b8664d29 to fix the arm64 build.
I'm fine with all your patches. I've queued them in my MMC series, and
will continue pushing them and / or merging them if it's ok for you.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170111/753e0bee/attachment.sig>
^ permalink raw reply
* [PATCH] tcb_clksrc: Use 32 bit tcb as sched_clock
From: David Engraf @ 2017-01-11 13:50 UTC (permalink / raw)
To: linux-arm-kernel
On newer boards the TC can be read as single 32 bit value without locking.
Thus the clock can be used as reference for sched_clock which is much more
accurate than the jiffies implementation.
Tested on a Atmel SAMA5D2 board.
Signed-off-by: David Engraf <david.engraf@sysgo.com>
---
drivers/clocksource/tcb_clksrc.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index d4ca996..745844e 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -10,6 +10,7 @@
#include <linux/io.h>
#include <linux/platform_device.h>
#include <linux/atmel_tc.h>
+#include <linux/sched_clock.h>
/*
@@ -56,11 +57,16 @@ static u64 tc_get_cycles(struct clocksource *cs)
return (upper << 16) | lower;
}
-static u64 tc_get_cycles32(struct clocksource *cs)
+static u32 tc_get_cv32(void)
{
return __raw_readl(tcaddr + ATMEL_TC_REG(0, CV));
}
+static u64 tc_get_cycles32(struct clocksource *cs)
+{
+ return tc_get_cv32();
+}
+
static struct clocksource clksrc = {
.name = "tcb_clksrc",
.rating = 200,
@@ -69,6 +75,11 @@ static struct clocksource clksrc = {
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
};
+static u64 notrace tc_read_sched_clock(void)
+{
+ return tc_get_cv32();
+}
+
#ifdef CONFIG_GENERIC_CLOCKEVENTS
struct tc_clkevt_device {
@@ -339,6 +350,9 @@ static int __init tcb_clksrc_init(void)
clksrc.read = tc_get_cycles32;
/* setup ony channel 0 */
tcb_setup_single_chan(tc, best_divisor_idx);
+
+ /* register sched_clock on chips with single 32 bit counter */
+ sched_clock_register(tc_read_sched_clock, 32, divided_rate);
} else {
/* tclib will give us three clocks no matter what the
* underlying platform supports.
--
2.9.3
^ permalink raw reply related
* [PATCH 1/2] crypto: mediatek - remove ARM dependencies
From: Arnd Bergmann @ 2017-01-11 13:50 UTC (permalink / raw)
To: linux-arm-kernel
Building the mediatek driver on an older ARM architecture results in a
harmless warning:
warning: (ARCH_OMAP2PLUS_TYPICAL && CRYPTO_DEV_MEDIATEK) selects NEON which has unmet direct dependencies (VFPv3 && CPU_V7)
We could add an explicit dependency on CPU_V7, but it seems nicer to
open up the build to additional configurations. This replaces the ARM
optimized algorithm selection with the normal one that all other drivers
use, and that in turn lets us relax the dependency on ARM and drop
a number of the unrelated 'select' statements.
Obviously a real user would still select those other optimized drivers
as a fallback, but as there is no strict dependency, we can leave that
up to the user.
Fixes: 785e5c616c84 ("crypto: mediatek - Add crypto driver support for some MediaTek chips")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/crypto/Kconfig | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 8ded3af88b16..9d37ae07b4ce 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -555,15 +555,12 @@ config CRYPTO_DEV_ROCKCHIP
config CRYPTO_DEV_MEDIATEK
tristate "MediaTek's EIP97 Cryptographic Engine driver"
- depends on ARM && (ARCH_MEDIATEK || COMPILE_TEST)
- select NEON
- select KERNEL_MODE_NEON
- select ARM_CRYPTO
+ depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST
select CRYPTO_AES
select CRYPTO_BLKCIPHER
- select CRYPTO_SHA1_ARM_NEON
- select CRYPTO_SHA256_ARM
- select CRYPTO_SHA512_ARM
+ select CRYPTO_SHA1
+ select CRYPTO_SHA256
+ select CRYPTO_SHA512
select CRYPTO_HMAC
help
This driver allows you to utilize the hardware crypto accelerator
--
2.9.0
^ permalink raw reply related
* [PATCHv5 3/8] rtc: add STM32 RTC driver
From: Amelie Delaunay @ 2017-01-11 13:46 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds support for the STM32 RTC.
Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
---
drivers/rtc/Kconfig | 11 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-stm32.c | 727 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 739 insertions(+)
create mode 100644 drivers/rtc/rtc-stm32.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index e859d14..11eb28a 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1706,6 +1706,17 @@ config RTC_DRV_PIC32
This driver can also be built as a module. If so, the module
will be called rtc-pic32
+config RTC_DRV_STM32
+ tristate "STM32 RTC"
+ select REGMAP_MMIO
+ depends on ARCH_STM32 || COMPILE_TEST
+ help
+ If you say yes here you get support for the STM32 On-Chip
+ Real Time Clock.
+
+ This driver can also be built as a module, if so, the module
+ will be called "rtc-stm32".
+
comment "HID Sensor RTC drivers"
config RTC_DRV_HID_SENSOR_TIME
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 1ac694a..87bd9cc 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -144,6 +144,7 @@ obj-$(CONFIG_RTC_DRV_SNVS) += rtc-snvs.o
obj-$(CONFIG_RTC_DRV_SPEAR) += rtc-spear.o
obj-$(CONFIG_RTC_DRV_STARFIRE) += rtc-starfire.o
obj-$(CONFIG_RTC_DRV_STK17TA8) += rtc-stk17ta8.o
+obj-$(CONFIG_RTC_DRV_STM32) += rtc-stm32.o
obj-$(CONFIG_RTC_DRV_STMP) += rtc-stmp3xxx.o
obj-$(CONFIG_RTC_DRV_ST_LPC) += rtc-st-lpc.o
obj-$(CONFIG_RTC_DRV_SUN4V) += rtc-sun4v.o
diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
new file mode 100644
index 0000000..c4789b5
--- /dev/null
+++ b/drivers/rtc/rtc-stm32.c
@@ -0,0 +1,727 @@
+/*
+ * Copyright (C) Amelie Delaunay 2016
+ * Author: Amelie Delaunay <amelie.delaunay@st.com>
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include <linux/bcd.h>
+#include <linux/clk.h>
+#include <linux/iopoll.h>
+#include <linux/ioport.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/rtc.h>
+
+#define DRIVER_NAME "stm32_rtc"
+
+/* STM32 RTC registers */
+#define STM32_RTC_TR 0x00
+#define STM32_RTC_DR 0x04
+#define STM32_RTC_CR 0x08
+#define STM32_RTC_ISR 0x0C
+#define STM32_RTC_PRER 0x10
+#define STM32_RTC_ALRMAR 0x1C
+#define STM32_RTC_WPR 0x24
+
+/* STM32_RTC_TR bit fields */
+#define STM32_RTC_TR_SEC_SHIFT 0
+#define STM32_RTC_TR_SEC GENMASK(6, 0)
+#define STM32_RTC_TR_MIN_SHIFT 8
+#define STM32_RTC_TR_MIN GENMASK(14, 8)
+#define STM32_RTC_TR_HOUR_SHIFT 16
+#define STM32_RTC_TR_HOUR GENMASK(21, 16)
+
+/* STM32_RTC_DR bit fields */
+#define STM32_RTC_DR_DATE_SHIFT 0
+#define STM32_RTC_DR_DATE GENMASK(5, 0)
+#define STM32_RTC_DR_MONTH_SHIFT 8
+#define STM32_RTC_DR_MONTH GENMASK(12, 8)
+#define STM32_RTC_DR_WDAY_SHIFT 13
+#define STM32_RTC_DR_WDAY GENMASK(15, 13)
+#define STM32_RTC_DR_YEAR_SHIFT 16
+#define STM32_RTC_DR_YEAR GENMASK(23, 16)
+
+/* STM32_RTC_CR bit fields */
+#define STM32_RTC_CR_FMT BIT(6)
+#define STM32_RTC_CR_ALRAE BIT(8)
+#define STM32_RTC_CR_ALRAIE BIT(12)
+
+/* STM32_RTC_ISR bit fields */
+#define STM32_RTC_ISR_ALRAWF BIT(0)
+#define STM32_RTC_ISR_INITS BIT(4)
+#define STM32_RTC_ISR_RSF BIT(5)
+#define STM32_RTC_ISR_INITF BIT(6)
+#define STM32_RTC_ISR_INIT BIT(7)
+#define STM32_RTC_ISR_ALRAF BIT(8)
+
+/* STM32_RTC_PRER bit fields */
+#define STM32_RTC_PRER_PRED_S_SHIFT 0
+#define STM32_RTC_PRER_PRED_S GENMASK(14, 0)
+#define STM32_RTC_PRER_PRED_A_SHIFT 16
+#define STM32_RTC_PRER_PRED_A GENMASK(22, 16)
+
+/* STM32_RTC_ALRMAR and STM32_RTC_ALRMBR bit fields */
+#define STM32_RTC_ALRMXR_SEC_SHIFT 0
+#define STM32_RTC_ALRMXR_SEC GENMASK(6, 0)
+#define STM32_RTC_ALRMXR_SEC_MASK BIT(7)
+#define STM32_RTC_ALRMXR_MIN_SHIFT 8
+#define STM32_RTC_ALRMXR_MIN GENMASK(14, 8)
+#define STM32_RTC_ALRMXR_MIN_MASK BIT(15)
+#define STM32_RTC_ALRMXR_HOUR_SHIFT 16
+#define STM32_RTC_ALRMXR_HOUR GENMASK(21, 16)
+#define STM32_RTC_ALRMXR_PM BIT(22)
+#define STM32_RTC_ALRMXR_HOUR_MASK BIT(23)
+#define STM32_RTC_ALRMXR_DATE_SHIFT 24
+#define STM32_RTC_ALRMXR_DATE GENMASK(29, 24)
+#define STM32_RTC_ALRMXR_WDSEL BIT(30)
+#define STM32_RTC_ALRMXR_WDAY_SHIFT 24
+#define STM32_RTC_ALRMXR_WDAY GENMASK(27, 24)
+#define STM32_RTC_ALRMXR_DATE_MASK BIT(31)
+
+/* STM32_RTC_WPR key constants */
+#define RTC_WPR_1ST_KEY 0xCA
+#define RTC_WPR_2ND_KEY 0x53
+#define RTC_WPR_WRONG_KEY 0xFF
+
+/*
+ * RTC registers are protected against parasitic write access.
+ * PWR_CR_DBP bit must be set to enable write access to RTC registers.
+ */
+/* STM32_PWR_CR */
+#define PWR_CR 0x00
+/* STM32_PWR_CR bit field */
+#define PWR_CR_DBP BIT(8)
+
+struct stm32_rtc {
+ struct rtc_device *rtc_dev;
+ void __iomem *base;
+ struct regmap *dbp;
+ struct clk *ck_rtc;
+ int irq_alarm;
+};
+
+static void stm32_rtc_wpr_unlock(struct stm32_rtc *rtc)
+{
+ writel_relaxed(RTC_WPR_1ST_KEY, rtc->base + STM32_RTC_WPR);
+ writel_relaxed(RTC_WPR_2ND_KEY, rtc->base + STM32_RTC_WPR);
+}
+
+static void stm32_rtc_wpr_lock(struct stm32_rtc *rtc)
+{
+ writel_relaxed(RTC_WPR_WRONG_KEY, rtc->base + STM32_RTC_WPR);
+}
+
+static int stm32_rtc_enter_init_mode(struct stm32_rtc *rtc)
+{
+ unsigned int isr = readl_relaxed(rtc->base + STM32_RTC_ISR);
+
+ if (!(isr & STM32_RTC_ISR_INITF)) {
+ isr |= STM32_RTC_ISR_INIT;
+ writel_relaxed(isr, rtc->base + STM32_RTC_ISR);
+
+ /*
+ * It takes around 2 ck_rtc clock cycles to enter in
+ * initialization phase mode (and have INITF flag set). As
+ * slowest ck_rtc frequency may be 32kHz and highest should be
+ * 1MHz, we poll every 10 us with a timeout of 100ms.
+ */
+ return readl_relaxed_poll_timeout_atomic(
+ rtc->base + STM32_RTC_ISR,
+ isr, (isr & STM32_RTC_ISR_INITF),
+ 10, 100000);
+ }
+
+ return 0;
+}
+
+static void stm32_rtc_exit_init_mode(struct stm32_rtc *rtc)
+{
+ unsigned int isr = readl_relaxed(rtc->base + STM32_RTC_ISR);
+
+ isr &= ~STM32_RTC_ISR_INIT;
+ writel_relaxed(isr, rtc->base + STM32_RTC_ISR);
+}
+
+static int stm32_rtc_wait_sync(struct stm32_rtc *rtc)
+{
+ unsigned int isr = readl_relaxed(rtc->base + STM32_RTC_ISR);
+
+ isr &= ~STM32_RTC_ISR_RSF;
+ writel_relaxed(isr, rtc->base + STM32_RTC_ISR);
+
+ /*
+ * Wait for RSF to be set to ensure the calendar registers are
+ * synchronised, it takes around 2 ck_rtc clock cycles
+ */
+ return readl_relaxed_poll_timeout_atomic(rtc->base + STM32_RTC_ISR,
+ isr,
+ (isr & STM32_RTC_ISR_RSF),
+ 10, 100000);
+}
+
+static irqreturn_t stm32_rtc_alarm_irq(int irq, void *dev_id)
+{
+ struct stm32_rtc *rtc = (struct stm32_rtc *)dev_id;
+ unsigned int isr, cr;
+
+ mutex_lock(&rtc->rtc_dev->ops_lock);
+
+ isr = readl_relaxed(rtc->base + STM32_RTC_ISR);
+ cr = readl_relaxed(rtc->base + STM32_RTC_CR);
+
+ if ((isr & STM32_RTC_ISR_ALRAF) &&
+ (cr & STM32_RTC_CR_ALRAIE)) {
+ /* Alarm A flag - Alarm interrupt */
+ dev_dbg(&rtc->rtc_dev->dev, "Alarm occurred\n");
+
+ /* Pass event to the kernel */
+ rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF);
+
+ /* Clear event flag, otherwise new events won't be received */
+ writel_relaxed(isr & ~STM32_RTC_ISR_ALRAF,
+ rtc->base + STM32_RTC_ISR);
+ }
+
+ mutex_unlock(&rtc->rtc_dev->ops_lock);
+
+ return IRQ_HANDLED;
+}
+
+/* Convert rtc_time structure from bin to bcd format */
+static void tm2bcd(struct rtc_time *tm)
+{
+ tm->tm_sec = bin2bcd(tm->tm_sec);
+ tm->tm_min = bin2bcd(tm->tm_min);
+ tm->tm_hour = bin2bcd(tm->tm_hour);
+
+ tm->tm_mday = bin2bcd(tm->tm_mday);
+ tm->tm_mon = bin2bcd(tm->tm_mon + 1);
+ tm->tm_year = bin2bcd(tm->tm_year - 100);
+ /*
+ * Number of days since Sunday
+ * - on kernel side, 0=Sunday...6=Saturday
+ * - on rtc side, 0=invalid,1=Monday...7=Sunday
+ */
+ tm->tm_wday = (!tm->tm_wday) ? 7 : tm->tm_wday;
+}
+
+/* Convert rtc_time structure from bcd to bin format */
+static void bcd2tm(struct rtc_time *tm)
+{
+ tm->tm_sec = bcd2bin(tm->tm_sec);
+ tm->tm_min = bcd2bin(tm->tm_min);
+ tm->tm_hour = bcd2bin(tm->tm_hour);
+
+ tm->tm_mday = bcd2bin(tm->tm_mday);
+ tm->tm_mon = bcd2bin(tm->tm_mon) - 1;
+ tm->tm_year = bcd2bin(tm->tm_year) + 100;
+ /*
+ * Number of days since Sunday
+ * - on kernel side, 0=Sunday...6=Saturday
+ * - on rtc side, 0=invalid,1=Monday...7=Sunday
+ */
+ tm->tm_wday %= 7;
+}
+
+static int stm32_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct stm32_rtc *rtc = dev_get_drvdata(dev);
+ unsigned int tr, dr;
+
+ /* Time and Date in BCD format */
+ tr = readl_relaxed(rtc->base + STM32_RTC_TR);
+ dr = readl_relaxed(rtc->base + STM32_RTC_DR);
+
+ tm->tm_sec = (tr & STM32_RTC_TR_SEC) >> STM32_RTC_TR_SEC_SHIFT;
+ tm->tm_min = (tr & STM32_RTC_TR_MIN) >> STM32_RTC_TR_MIN_SHIFT;
+ tm->tm_hour = (tr & STM32_RTC_TR_HOUR) >> STM32_RTC_TR_HOUR_SHIFT;
+
+ tm->tm_mday = (dr & STM32_RTC_DR_DATE) >> STM32_RTC_DR_DATE_SHIFT;
+ tm->tm_mon = (dr & STM32_RTC_DR_MONTH) >> STM32_RTC_DR_MONTH_SHIFT;
+ tm->tm_year = (dr & STM32_RTC_DR_YEAR) >> STM32_RTC_DR_YEAR_SHIFT;
+ tm->tm_wday = (dr & STM32_RTC_DR_WDAY) >> STM32_RTC_DR_WDAY_SHIFT;
+
+ /* We don't report tm_yday and tm_isdst */
+
+ bcd2tm(tm);
+
+ return 0;
+}
+
+static int stm32_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct stm32_rtc *rtc = dev_get_drvdata(dev);
+ unsigned int tr, dr;
+ int ret = 0;
+
+ tm2bcd(tm);
+
+ /* Time in BCD format */
+ tr = ((tm->tm_sec << STM32_RTC_TR_SEC_SHIFT) & STM32_RTC_TR_SEC) |
+ ((tm->tm_min << STM32_RTC_TR_MIN_SHIFT) & STM32_RTC_TR_MIN) |
+ ((tm->tm_hour << STM32_RTC_TR_HOUR_SHIFT) & STM32_RTC_TR_HOUR);
+
+ /* Date in BCD format */
+ dr = ((tm->tm_mday << STM32_RTC_DR_DATE_SHIFT) & STM32_RTC_DR_DATE) |
+ ((tm->tm_mon << STM32_RTC_DR_MONTH_SHIFT) & STM32_RTC_DR_MONTH) |
+ ((tm->tm_year << STM32_RTC_DR_YEAR_SHIFT) & STM32_RTC_DR_YEAR) |
+ ((tm->tm_wday << STM32_RTC_DR_WDAY_SHIFT) & STM32_RTC_DR_WDAY);
+
+ stm32_rtc_wpr_unlock(rtc);
+
+ ret = stm32_rtc_enter_init_mode(rtc);
+ if (ret) {
+ dev_err(dev, "Can't enter in init mode. Set time aborted.\n");
+ goto end;
+ }
+
+ writel_relaxed(tr, rtc->base + STM32_RTC_TR);
+ writel_relaxed(dr, rtc->base + STM32_RTC_DR);
+
+ stm32_rtc_exit_init_mode(rtc);
+
+ ret = stm32_rtc_wait_sync(rtc);
+end:
+ stm32_rtc_wpr_lock(rtc);
+
+ return ret;
+}
+
+static int stm32_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct stm32_rtc *rtc = dev_get_drvdata(dev);
+ struct rtc_time *tm = &alrm->time;
+ unsigned int alrmar, cr, isr;
+
+ alrmar = readl_relaxed(rtc->base + STM32_RTC_ALRMAR);
+ cr = readl_relaxed(rtc->base + STM32_RTC_CR);
+ isr = readl_relaxed(rtc->base + STM32_RTC_ISR);
+
+ if (alrmar & STM32_RTC_ALRMXR_DATE_MASK) {
+ /*
+ * Date/day doesn't matter in Alarm comparison so alarm
+ * triggers every day
+ */
+ tm->tm_mday = -1;
+ tm->tm_wday = -1;
+ } else {
+ if (alrmar & STM32_RTC_ALRMXR_WDSEL) {
+ /* Alarm is set to a day of week */
+ tm->tm_mday = -1;
+ tm->tm_wday = (alrmar & STM32_RTC_ALRMXR_WDAY) >>
+ STM32_RTC_ALRMXR_WDAY_SHIFT;
+ tm->tm_wday %= 7;
+ } else {
+ /* Alarm is set to a day of month */
+ tm->tm_wday = -1;
+ tm->tm_mday = (alrmar & STM32_RTC_ALRMXR_DATE) >>
+ STM32_RTC_ALRMXR_DATE_SHIFT;
+ }
+ }
+
+ if (alrmar & STM32_RTC_ALRMXR_HOUR_MASK) {
+ /* Hours don't matter in Alarm comparison */
+ tm->tm_hour = -1;
+ } else {
+ tm->tm_hour = (alrmar & STM32_RTC_ALRMXR_HOUR) >>
+ STM32_RTC_ALRMXR_HOUR_SHIFT;
+ if (alrmar & STM32_RTC_ALRMXR_PM)
+ tm->tm_hour += 12;
+ }
+
+ if (alrmar & STM32_RTC_ALRMXR_MIN_MASK) {
+ /* Minutes don't matter in Alarm comparison */
+ tm->tm_min = -1;
+ } else {
+ tm->tm_min = (alrmar & STM32_RTC_ALRMXR_MIN) >>
+ STM32_RTC_ALRMXR_MIN_SHIFT;
+ }
+
+ if (alrmar & STM32_RTC_ALRMXR_SEC_MASK) {
+ /* Seconds don't matter in Alarm comparison */
+ tm->tm_sec = -1;
+ } else {
+ tm->tm_sec = (alrmar & STM32_RTC_ALRMXR_SEC) >>
+ STM32_RTC_ALRMXR_SEC_SHIFT;
+ }
+
+ bcd2tm(tm);
+
+ alrm->enabled = (cr & STM32_RTC_CR_ALRAE) ? 1 : 0;
+ alrm->pending = (isr & STM32_RTC_ISR_ALRAF) ? 1 : 0;
+
+ return 0;
+}
+
+static int stm32_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+ struct stm32_rtc *rtc = dev_get_drvdata(dev);
+ unsigned int isr, cr;
+
+ cr = readl_relaxed(rtc->base + STM32_RTC_CR);
+
+ stm32_rtc_wpr_unlock(rtc);
+
+ /* We expose Alarm A to the kernel */
+ if (enabled)
+ cr |= (STM32_RTC_CR_ALRAIE | STM32_RTC_CR_ALRAE);
+ else
+ cr &= ~(STM32_RTC_CR_ALRAIE | STM32_RTC_CR_ALRAE);
+ writel_relaxed(cr, rtc->base + STM32_RTC_CR);
+
+ /* Clear event flag, otherwise new events won't be received */
+ isr = readl_relaxed(rtc->base + STM32_RTC_ISR);
+ isr &= ~STM32_RTC_ISR_ALRAF;
+ writel_relaxed(isr, rtc->base + STM32_RTC_ISR);
+
+ stm32_rtc_wpr_lock(rtc);
+
+ return 0;
+}
+
+static int stm32_rtc_valid_alrm(struct stm32_rtc *rtc, struct rtc_time *tm)
+{
+ unsigned int cur_day, cur_mon, cur_year, cur_hour, cur_min, cur_sec;
+ unsigned int dr = readl_relaxed(rtc->base + STM32_RTC_DR);
+ unsigned int tr = readl_relaxed(rtc->base + STM32_RTC_TR);
+
+ cur_day = (dr & STM32_RTC_DR_DATE) >> STM32_RTC_DR_DATE_SHIFT;
+ cur_mon = (dr & STM32_RTC_DR_MONTH) >> STM32_RTC_DR_MONTH_SHIFT;
+ cur_year = (dr & STM32_RTC_DR_YEAR) >> STM32_RTC_DR_YEAR_SHIFT;
+ cur_sec = (tr & STM32_RTC_TR_SEC) >> STM32_RTC_TR_SEC_SHIFT;
+ cur_min = (tr & STM32_RTC_TR_MIN) >> STM32_RTC_TR_MIN_SHIFT;
+ cur_hour = (tr & STM32_RTC_TR_HOUR) >> STM32_RTC_TR_HOUR_SHIFT;
+
+ /*
+ * Assuming current date is M-D-Y H:M:S.
+ * RTC alarm can't be set on a specific month and year.
+ * So the valid alarm range is:
+ * M-D-Y H:M:S < alarm <= (M+1)-D-Y H:M:S
+ * with a specific case for December...
+ */
+ if ((((tm->tm_year > cur_year) &&
+ (tm->tm_mon == 0x1) && (cur_mon == 0x12)) ||
+ ((tm->tm_year == cur_year) &&
+ (tm->tm_mon <= cur_mon + 1))) &&
+ ((tm->tm_mday > cur_day) ||
+ ((tm->tm_mday == cur_day) &&
+ ((tm->tm_hour > cur_hour) ||
+ ((tm->tm_hour == cur_hour) && (tm->tm_min > cur_min)) ||
+ ((tm->tm_hour == cur_hour) && (tm->tm_min == cur_min) &&
+ (tm->tm_sec >= cur_sec))))))
+ return 0;
+
+ return -EINVAL;
+}
+
+static int stm32_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct stm32_rtc *rtc = dev_get_drvdata(dev);
+ struct rtc_time *tm = &alrm->time;
+ unsigned int cr, isr, alrmar;
+ int ret = 0;
+
+ tm2bcd(tm);
+
+ /*
+ * RTC alarm can't be set on a specific date, unless this date is
+ * up to the same day of month next month.
+ */
+ if (stm32_rtc_valid_alrm(rtc, tm) < 0) {
+ dev_err(dev, "Alarm can be set only on upcoming month.\n");
+ return -EINVAL;
+ }
+
+ alrmar = 0;
+ /* tm_year and tm_mon are not used because not supported by RTC */
+ alrmar |= (tm->tm_mday << STM32_RTC_ALRMXR_DATE_SHIFT) &
+ STM32_RTC_ALRMXR_DATE;
+ /* 24-hour format */
+ alrmar &= ~STM32_RTC_ALRMXR_PM;
+ alrmar |= (tm->tm_hour << STM32_RTC_ALRMXR_HOUR_SHIFT) &
+ STM32_RTC_ALRMXR_HOUR;
+ alrmar |= (tm->tm_min << STM32_RTC_ALRMXR_MIN_SHIFT) &
+ STM32_RTC_ALRMXR_MIN;
+ alrmar |= (tm->tm_sec << STM32_RTC_ALRMXR_SEC_SHIFT) &
+ STM32_RTC_ALRMXR_SEC;
+
+ stm32_rtc_wpr_unlock(rtc);
+
+ /* Disable Alarm */
+ cr = readl_relaxed(rtc->base + STM32_RTC_CR);
+ cr &= ~STM32_RTC_CR_ALRAE;
+ writel_relaxed(cr, rtc->base + STM32_RTC_CR);
+
+ /*
+ * Poll Alarm write flag to be sure that Alarm update is allowed: it
+ * takes around 2 ck_rtc clock cycles
+ */
+ ret = readl_relaxed_poll_timeout_atomic(rtc->base + STM32_RTC_ISR,
+ isr,
+ (isr & STM32_RTC_ISR_ALRAWF),
+ 10, 100000);
+
+ if (ret) {
+ dev_err(dev, "Alarm update not allowed\n");
+ goto end;
+ }
+
+ /* Write to Alarm register */
+ writel_relaxed(alrmar, rtc->base + STM32_RTC_ALRMAR);
+
+ if (alrm->enabled)
+ stm32_rtc_alarm_irq_enable(dev, 1);
+ else
+ stm32_rtc_alarm_irq_enable(dev, 0);
+
+end:
+ stm32_rtc_wpr_lock(rtc);
+
+ return ret;
+}
+
+static const struct rtc_class_ops stm32_rtc_ops = {
+ .read_time = stm32_rtc_read_time,
+ .set_time = stm32_rtc_set_time,
+ .read_alarm = stm32_rtc_read_alarm,
+ .set_alarm = stm32_rtc_set_alarm,
+ .alarm_irq_enable = stm32_rtc_alarm_irq_enable,
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id stm32_rtc_of_match[] = {
+ { .compatible = "st,stm32-rtc" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, stm32_rtc_of_match);
+#endif
+
+static int stm32_rtc_init(struct platform_device *pdev,
+ struct stm32_rtc *rtc)
+{
+ unsigned int prer, pred_a, pred_s, pred_a_max, pred_s_max, cr;
+ unsigned int rate;
+ int ret = 0;
+
+ rate = clk_get_rate(rtc->ck_rtc);
+
+ /* Find prediv_a and prediv_s to obtain the 1Hz calendar clock */
+ pred_a_max = STM32_RTC_PRER_PRED_A >> STM32_RTC_PRER_PRED_A_SHIFT;
+ pred_s_max = STM32_RTC_PRER_PRED_S >> STM32_RTC_PRER_PRED_S_SHIFT;
+
+ for (pred_a = pred_a_max; pred_a >= 0; pred_a--) {
+ pred_s = (rate / (pred_a + 1)) - 1;
+
+ if (((pred_s + 1) * (pred_a + 1)) == rate)
+ break;
+ }
+
+ /*
+ * Can't find a 1Hz, so give priority to RTC power consumption
+ * by choosing the higher possible value for prediv_a
+ */
+ if ((pred_s > pred_s_max) || (pred_a > pred_a_max)) {
+ pred_a = pred_a_max;
+ pred_s = (rate / (pred_a + 1)) - 1;
+
+ dev_warn(&pdev->dev, "ck_rtc is %s\n",
+ (rate - ((pred_a + 1) * (pred_s + 1)) < 0) ?
+ "fast" : "slow");
+ }
+
+ stm32_rtc_wpr_unlock(rtc);
+
+ ret = stm32_rtc_enter_init_mode(rtc);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Can't enter in init mode. Prescaler config failed.\n");
+ goto end;
+ }
+
+ prer = (pred_s << STM32_RTC_PRER_PRED_S_SHIFT) & STM32_RTC_PRER_PRED_S;
+ writel_relaxed(prer, rtc->base + STM32_RTC_PRER);
+ prer |= (pred_a << STM32_RTC_PRER_PRED_A_SHIFT) & STM32_RTC_PRER_PRED_A;
+ writel_relaxed(prer, rtc->base + STM32_RTC_PRER);
+
+ /* Force 24h time format */
+ cr = readl_relaxed(rtc->base + STM32_RTC_CR);
+ cr &= ~STM32_RTC_CR_FMT;
+ writel_relaxed(cr, rtc->base + STM32_RTC_CR);
+
+ stm32_rtc_exit_init_mode(rtc);
+
+ ret = stm32_rtc_wait_sync(rtc);
+end:
+ stm32_rtc_wpr_lock(rtc);
+
+ return ret;
+}
+
+static int stm32_rtc_probe(struct platform_device *pdev)
+{
+ struct stm32_rtc *rtc;
+ struct resource *res;
+ int ret;
+
+ rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
+ if (!rtc)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ rtc->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(rtc->base))
+ return PTR_ERR(rtc->base);
+
+ rtc->dbp = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+ "st,syscfg");
+ if (IS_ERR(rtc->dbp)) {
+ dev_err(&pdev->dev, "no st,syscfg\n");
+ return PTR_ERR(rtc->dbp);
+ }
+
+ rtc->ck_rtc = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(rtc->ck_rtc)) {
+ dev_err(&pdev->dev, "no ck_rtc clock");
+ return PTR_ERR(rtc->ck_rtc);
+ }
+
+ ret = clk_prepare_enable(rtc->ck_rtc);
+ if (ret)
+ return ret;
+
+ regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, PWR_CR_DBP);
+
+ /*
+ * After a system reset, RTC_ISR.INITS flag can be read to check if
+ * the calendar has been initalized or not. INITS flag is reset by a
+ * power-on reset (no vbat, no power-supply). It is not reset if
+ * ck_rtc parent clock has changed (so RTC prescalers need to be
+ * changed). That's why we cannot rely on this flag to know if RTC
+ * init has to be done.
+ */
+ ret = stm32_rtc_init(pdev, rtc);
+ if (ret)
+ goto err;
+
+ rtc->irq_alarm = platform_get_irq(pdev, 0);
+ if (rtc->irq_alarm <= 0) {
+ dev_err(&pdev->dev, "no alarm irq\n");
+ ret = rtc->irq_alarm;
+ goto err;
+ }
+
+ platform_set_drvdata(pdev, rtc);
+
+ ret = device_init_wakeup(&pdev->dev, true);
+ if (ret)
+ dev_warn(&pdev->dev,
+ "alarm won't be able to wake up the system");
+
+ rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, pdev->name,
+ &stm32_rtc_ops, THIS_MODULE);
+ if (IS_ERR(rtc->rtc_dev)) {
+ ret = PTR_ERR(rtc->rtc_dev);
+ dev_err(&pdev->dev, "rtc device registration failed, err=%d\n",
+ ret);
+ goto err;
+ }
+
+ /* Handle RTC alarm interrupts */
+ ret = devm_request_threaded_irq(&pdev->dev, rtc->irq_alarm, NULL,
+ stm32_rtc_alarm_irq,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ pdev->name, rtc);
+ if (ret) {
+ dev_err(&pdev->dev, "IRQ%d (alarm interrupt) already claimed\n",
+ rtc->irq_alarm);
+ goto err;
+ }
+
+ /*
+ * If INITS flag is reset (calendar year field set to 0x00), calendar
+ * must be initialized
+ */
+ if (!(readl_relaxed(rtc->base + STM32_RTC_ISR) & STM32_RTC_ISR_INITS))
+ dev_warn(&pdev->dev, "Date/Time must be initialized\n");
+
+ return 0;
+err:
+ clk_disable_unprepare(rtc->ck_rtc);
+
+ regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, ~PWR_CR_DBP);
+
+ device_init_wakeup(&pdev->dev, false);
+
+ return ret;
+}
+
+static int __exit stm32_rtc_remove(struct platform_device *pdev)
+{
+ struct stm32_rtc *rtc = platform_get_drvdata(pdev);
+ unsigned int cr;
+
+ /* Disable interrupts */
+ stm32_rtc_wpr_unlock(rtc);
+ cr = readl_relaxed(rtc->base + STM32_RTC_CR);
+ cr &= ~STM32_RTC_CR_ALRAIE;
+ writel_relaxed(cr, rtc->base + STM32_RTC_CR);
+ stm32_rtc_wpr_lock(rtc);
+
+ clk_disable_unprepare(rtc->ck_rtc);
+
+ /* Enable backup domain write protection */
+ regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, ~PWR_CR_DBP);
+
+ device_init_wakeup(&pdev->dev, false);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int stm32_rtc_suspend(struct device *dev)
+{
+ struct stm32_rtc *rtc = dev_get_drvdata(dev);
+
+ if (device_may_wakeup(dev))
+ return enable_irq_wake(rtc->irq_alarm);
+
+ return 0;
+}
+
+static int stm32_rtc_resume(struct device *dev)
+{
+ struct stm32_rtc *rtc = dev_get_drvdata(dev);
+ int ret = 0;
+
+ ret = stm32_rtc_wait_sync(rtc);
+ if (ret < 0)
+ return ret;
+
+ if (device_may_wakeup(dev))
+ return disable_irq_wake(rtc->irq_alarm);
+
+ return ret;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(stm32_rtc_pm_ops,
+ stm32_rtc_suspend, stm32_rtc_resume);
+
+static struct platform_driver stm32_rtc_driver = {
+ .probe = stm32_rtc_probe,
+ .remove = stm32_rtc_remove,
+ .driver = {
+ .name = DRIVER_NAME,
+ .pm = &stm32_rtc_pm_ops,
+ .of_match_table = stm32_rtc_of_match,
+ },
+};
+
+module_platform_driver(stm32_rtc_driver);
+
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_AUTHOR("Amelie Delaunay <amelie.delaunay@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32 Real Time Clock driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related
* [PATCH] ARM: mv78xx0: fix possible PCI buffer overflow
From: Arnd Bergmann @ 2017-01-11 13:41 UTC (permalink / raw)
To: linux-arm-kernel
gcc-7.0 reports a potential array overflow:
arch/arm/mach-mv78xx0/pcie.c: In function 'mv78xx0_pcie_preinit':
arch/arm/mach-mv78xx0/pcie.c:81:4: error: output may be truncated before the last format character [-Werror=format-truncation=]
I haven't checked if this can actually happen, but making the
array one 32-bit word longer addresses the warning and makes
it completely safe.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/mach-mv78xx0/pcie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-mv78xx0/pcie.c b/arch/arm/mach-mv78xx0/pcie.c
index 13a7d72ee0c4..81ff4327a962 100644
--- a/arch/arm/mach-mv78xx0/pcie.c
+++ b/arch/arm/mach-mv78xx0/pcie.c
@@ -29,7 +29,7 @@ struct pcie_port {
u8 root_bus_nr;
void __iomem *base;
spinlock_t conf_lock;
- char mem_space_name[16];
+ char mem_space_name[20];
struct resource res;
};
--
2.9.0
^ permalink raw reply related
* [PATCH] arm64: avoid increasing DMA masks above what hardware supports
From: Nikita Yushchenko @ 2017-01-11 13:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57459a4a-2c57-e081-8f27-cb83f23b5815@arm.com>
> Yes, I think that ought to work, although the __iommu_setup_dma_ops()
> call will still want a real size reflecting the default mask
I see iommu_dma_ops do not define set_dma_mask.
So what if setup was done for size reflecting one mask and then driver
changes mask? Will things still operate correctly?
^ permalink raw reply
* [PATCH] clk: stm32f4: avoid uninitialized variable access
From: Arnd Bergmann @ 2017-01-11 13:40 UTC (permalink / raw)
To: linux-arm-kernel
The failure path in the newly added function tries to free an
uninitialized pointer:
drivers/clk/clk-stm32f4.c: In function 'stm32f4_rcc_init':
drivers/clk/clk-stm32f4.c:1106:4: error: 'gate' may be used uninitialized in this function [-Werror=maybe-uninitialized]
I'm adding an initialization to NULL here to make the kfree()
succeed, and I'm also rearranging the cleanup so that the
same kfree() is used for any error path, making the function
slightly more robust against newly introduced bugs in the
error handling.
Fixes: daf2d117cbca ("clk: stm32f4: Add lcd-tft clock")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/clk/clk-stm32f4.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index 42f8534996de..4a3bb6e63525 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -1080,7 +1080,7 @@ static struct clk_hw *stm32_register_aux_clk(const char *name,
unsigned long flags, spinlock_t *lock)
{
struct clk_hw *hw;
- struct clk_gate *gate;
+ struct clk_gate *gate = NULL;
struct clk_mux *mux = NULL;
struct clk_hw *mux_hw = NULL, *gate_hw = NULL;
const struct clk_ops *mux_ops = NULL, *gate_ops = NULL;
@@ -1103,7 +1103,6 @@ static struct clk_hw *stm32_register_aux_clk(const char *name,
if (offset_mux != NO_MUX) {
mux = kzalloc(sizeof(*mux), GFP_KERNEL);
if (!mux) {
- kfree(gate);
hw = ERR_PTR(-EINVAL);
goto fail;
}
@@ -1116,8 +1115,10 @@ static struct clk_hw *stm32_register_aux_clk(const char *name,
mux_ops = &clk_mux_ops;
}
- if (mux_hw == NULL && gate_hw == NULL)
- return ERR_PTR(-EINVAL);
+ if (mux_hw == NULL && gate_hw == NULL) {
+ hw = ERR_PTR(-EINVAL);
+ goto fail;
+ }
hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
mux_hw, mux_ops,
@@ -1125,11 +1126,12 @@ static struct clk_hw *stm32_register_aux_clk(const char *name,
gate_hw, gate_ops,
flags);
+fail:
if (IS_ERR(hw)) {
kfree(gate);
kfree(mux);
}
-fail:
+
return hw;
}
--
2.9.0
^ permalink raw reply related
* [PATCHv4 3/8] rtc: add STM32 RTC driver
From: Amelie DELAUNAY @ 2017-01-11 13:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170111130438.GA9327@Red>
Hi Corentin,
Thanks for reviewing,
On 01/11/2017 02:04 PM, Corentin Labbe wrote:
> On Wed, Jan 11, 2017 at 01:48:25PM +0100, Amelie Delaunay wrote:
>> This patch adds support for the STM32 RTC.
>>
>> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
>> ---
>> drivers/rtc/Kconfig | 11 +
>> drivers/rtc/Makefile | 1 +
>> drivers/rtc/rtc-stm32.c | 727 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 739 insertions(+)
>> create mode 100644 drivers/rtc/rtc-stm32.c
>
> [...]
>> +/* STM32_PWR_CR */
>> +#define PWR_CR 0x00
>> +/* STM32_PWR_CR bit field */
>> +#define PWR_CR_DBP BIT(8)
>> +
>> +static struct regmap *dbp;
>
> Hello
>
> Why using a global static struct ?
> You could alloc a private structure in probe for storing it and use platform_set_drvdata()
>
This is to stay closer to how this backup domain protection is managed
in clk-stm32f4 driver, but I realize that I haven't the same
constraints. I'll move this struct in my stm32_rtc private structure.
> Regards
>
Regards
^ permalink raw reply
* [PATCH v2 6/6] arm64: allwinner: a64: Increase the MMC max frequency
From: Maxime Ripard @ 2017-01-11 13:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v64vU5soFybaurCKu1HE9FNo=-NGiy33ZVeoR6E7m4kk6w@mail.gmail.com>
Hi,
On Wed, Jan 11, 2017 at 02:44:00PM +0800, Chen-Yu Tsai wrote:
> On Wed, Jan 11, 2017 at 3:15 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi,
> >
> > On Tue, Jan 10, 2017 at 01:01:20AM +0800, Chen-Yu Tsai wrote:
> >> On Tue, Jan 10, 2017 at 12:46 AM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > All the controllers can have a maximum frequency of 200MHz.
> >> >
> >> > Since older SoCs cannot go that high, we cannot change the default maximum
> >> > frequency, but fortunately for us we have a property for that in the DT.
> >> >
> >> > This also has the side effect of allowing to use the MMC HS200 mode for the
> >> > boards that support it (with either 1.2v or 1.8v IOs).
> >> >
> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> > ---
> >> > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 3 +++
> >> > 1 file changed, 3 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> >> > index 8e149498e096..f46ae965cf5b 100644
> >> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> >> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> >> > @@ -332,6 +332,7 @@
> >> > resets = <&ccu RST_BUS_MMC0>;
> >> > reset-names = "ahb";
> >> > interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
> >> > + max-frequency = <200000000>;
> >>
> >> You also have to set one of MMC_CAP2_HS200* in the driver,
> >> or mmc-hs200-1_8v or mmc-hs200-1_2v in the device tree to
> >> actually use HS200, right?
> >
> > Yes, but that requires a board with 1.8V IOs to work properly, which
> > not all board use, so it's probably best to enable it in the board
> > DTS.
>
> It's limited by the vqmmc regulator. Either way the host controller
> supports it right?
Yes, but if the card supports HS200 with 1.8V, and the controller
reports it too, the core will pick that mode and will try to switch to
it, which in turn will fail, making the card initialisation fail as
well.
We basically have two choices: either we ask all the boards that
support it to set mmc-hs200-1_8v in their DTS, or we set it in the
DTSI and we have the boards set no-1-8-v in their DTS if they do not
support those modes.
The first case is just the more convenient, because so far we've only
seen one non-upstream (yet) board that supports 1.8V IOs. All the
other do not.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170111/8ea388ee/attachment-0001.sig>
^ permalink raw reply
* [PATCH] firmware: ti_sci: fix strncat length check
From: Arnd Bergmann @ 2017-01-11 13:37 UTC (permalink / raw)
To: linux-arm-kernel
gcc-7 notices that the length we pass to strncat is wrong:
drivers/firmware/ti_sci.c: In function 'ti_sci_probe':
drivers/firmware/ti_sci.c:204:32: error: specified bound 50 equals the size of the destination [-Werror=stringop-overflow=]
Instead of the total length, we must pass the length of the
remaining space here.
Fixes: aa276781a64a ("firmware: Add basic support for TI System Control Interface (TI-SCI) protocol")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/firmware/ti_sci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 874ff32db366..00cfed3c3e1a 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -202,7 +202,8 @@ static int ti_sci_debugfs_create(struct platform_device *pdev,
info->debug_buffer[info->debug_region_size] = 0;
info->d = debugfs_create_file(strncat(debug_name, dev_name(dev),
- sizeof(debug_name)),
+ sizeof(debug_name) -
+ sizeof("ti_sci_debug@")),
0444, NULL, info, &ti_sci_debug_fops);
if (IS_ERR(info->d))
return PTR_ERR(info->d);
--
2.9.0
^ permalink raw reply related
* [PATCH] ARM: defconfig: qcom: add APQ8060 DragonBoard devices
From: Linus Walleij @ 2017-01-11 13:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110145348.GD5710@hector.attlocal.net>
On Tue, Jan 10, 2017 at 3:53 PM, Andy Gross <andy.gross@linaro.org> wrote:
> On Tue, Jan 10, 2017 at 10:55:21AM +0100, Linus Walleij wrote:
>> This default-enables the devices found on the APQ8060 DragonBoard
>> in the qcom_defconfig:
>>
>> - EBI2 bus
>> - SMSC911x ethernet
>> - LEDs class and PM8058 LEDs driver, trigger and heartbeat
>> trigger (so we get heartbeat on the board by default)
>> - IIO framework, including the HRTimer trigger, KXSD9
>> accelerometer, MPU3050 gyroscope, AK8975 magnetometer and
>> BMP085 pressure sensor
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> This brings up a point of discussion. Do we even need the qcom_defconfig any
> more? Is everyone comfortable with using the multi_v7_defconfig?
>
> Aside from size of the image, i can't think of any other reason to keep around
> the separate qcom file.
Actually a bit of Arnd/Olof question.
Bystander opinion below:
That is pretty much up to the maintainer (you) I guess.
Reasons would be:
- Lower footprint (because you may not need all stuff selected
as 'y' compiled-in in multi_v7) on some platforms this is even
necessary to get a bootable image or one that will load in
reasonable time.
- Enable a few things by default (both compiled-in and modules)
that multi_v7 would consider to be littering
- For "my" systems I usually like them because these defconfigs
have vastly shorter compile time (because so much stuff that
idon't concern me is left out).
On the other hand: some ARMv7 system maintainers have x86
ambitions: compile once, run everywhere, and certainly that is
the ambition with multi_v7, and if that overshadows all the above,
just kill off qcom_defconfig and be happy :)
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 1/3] ARM: at91: flush the L2 cache before entering cpu idle
From: Jean-Jacques Hiblot @ 2017-01-11 13:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170111111814.GJ14217@n2100.armlinux.org.uk>
2017-01-11 12:18 GMT+01:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Wed, Jan 11, 2017 at 12:05:05PM +0100, Jean-Jacques Hiblot wrote:
>> 2017-01-11 9:15 GMT+01:00 <Wenyou.Yang@microchip.com>:
>> > Hi Jean-Jacques,
>> >
>> >> -----Original Message-----
>> >> From: Jean-Jacques Hiblot [mailto:jjhiblot at gmail.com]
>> >> Sent: 2017?1?11? 0:51
>> >> To: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> >> Cc: Wenyou Yang - A41535 <Wenyou.Yang@microchip.com>; Mark Rutland
>> >> <mark.rutland@arm.com>; devicetree <devicetree@vger.kernel.org>; Russell
>> >> King <linux@arm.linux.org.uk>; Wenyou Yang - A41535
>> >> <Wenyou.Yang@microchip.com>; Nicolas Ferre <nicolas.ferre@atmel.com>;
>> >> Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Rob Herring
>> >> <robh+dt@kernel.org>; linux-arm-kernel at lists.infradead.org
>> >> Subject: Re: [PATCH 1/3] ARM: at91: flush the L2 cache before entering cpu idle
>> >>
>> >> 2017-01-10 17:18 GMT+01:00 Alexandre Belloni
>> >> <alexandre.belloni@free-electrons.com>:
>> >> > I though a bit more about it, and I don't really like the new
>> >> > compatible string. I don't feel this should be necessary.
>> >> >
>> >> > What about the following:
>> >> >
>> >> > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c index
>> >> > b4332b727e9c..0333aca63e44 100644
>> >> > --- a/arch/arm/mach-at91/pm.c
>> >> > +++ b/arch/arm/mach-at91/pm.c
>> >> > @@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void); static
>> >> > struct {
>> >> > unsigned long uhp_udp_mask;
>> >> > int memctrl;
>> >> > + bool has_l2_cache;
>> >> > } at91_pm_data;
>> >> >
>> >> > void __iomem *at91_ramc_base[2];
>> >> > @@ -267,6 +268,11 @@ static void at91_ddr_standby(void)
>> >> > u32 lpr0, lpr1 = 0;
>> >> > u32 saved_lpr0, saved_lpr1 = 0;
>> >> >
>> >>
>> >> > + if (at91_pm_data.has_l2_cache) {
>> >> > + flush_cache_all();
>> >> what is the point of calling flush_cache_all() here ? Do we really care that dirty
>> >> data in L1 is written to DDR ? I may be missing something but to me it's just extra
>> >> latency.
>> >
>> > Are you mean use outer_flush_all() to flush all cache lines in the outer cache only?
>>
>> Yes that's what I meant. You see, you don't flush the cache for
>> sama5d3 so it shouldn't be required either for sam5d4. You should be
>> able to test it quickly and see if L1 flush is indeed required by
>> replacing flush_cache_all() with outer_flush_all(). BTW is highly
>> probable that L2 cache flush is done in outer_disable() so calling
>> outer_flush_all() is probably no required.
>
> Please don't. Read the comments in the code, and understand the APIs
> that you're suggesting people use _before_ making the suggestion:
>
> /**
> * outer_flush_all - clean and invalidate all cache lines in the outer cache
> *
> * Note: depending on implementation, this may not be atomic - it must
> * only be called with interrupts disabled and no other active outer
> * cache masters.
> *
> * It is intended that this function is only used by implementations
> * needing to override the outer_cache.disable() method due to security.
> * (Some implementations perform this as a clean followed by an invalidate.)
> */
>
> So, outer_flush_all() should not be called except from L2 cache code
> implementing the outer_disable() function - it's not intended for
> platforms to use.
OK. My bad. I didn't understand the comments.
>
> There are, however, sadly three users of outer_flush_all() which have
> crept in through arm-soc, that should be outer_disable() instead.
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
^ permalink raw reply
* [RFC PATCH v4 0/5] ARM: Fix dma_alloc_coherent() and friends for NOMMU
From: Benjamin Gaignard @ 2017-01-11 13:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484057925-23586-1-git-send-email-vladimir.murzin@arm.com>
2017-01-10 15:18 GMT+01:00 Vladimir Murzin <vladimir.murzin@arm.com>:
> Hi,
>
> It seem that addition of cache support for M-class cpus uncovered
> latent bug in DMA usage. NOMMU memory model has been treated as being
> always consistent; however, for R/M classes of cpu memory can be
> covered by MPU which in turn might configure RAM as Normal
> i.e. bufferable and cacheable. It breaks dma_alloc_coherent() and
> friends, since data can stuck in caches now or be buffered.
>
> This patch set is trying to address the issue by providing region of
> memory suitable for consistent DMA operations. It is supposed that
> such region is marked by MPU as non-cacheable. Robin suggested to
> advertise such memory as reserved shared-dma-pool, rather then using
> homebrew command line option, and extend dma-coherent to provide
> default DMA area in the similar way as it is done for CMA (PATCH
> 2/5). It allows us to offload all bookkeeping on generic coherent DMA
> framework, and it is seems that it might be reused by other
> architectures like c6x and blackfin.
>
> Dedicated DMA region is required for cases other than:
> - MMU/MPU is off
> - cpu is v7m w/o cache support
> - device is coherent
>
> In case one of the above conditions is true dma operations are forced
> to be coherent and wired with dma_noop_ops.
>
> To make life easier NOMMU dma operations are kept in separate
> compilation unit.
>
> Since the issue was reported in the same time as Benjamin sent his
> patch [1] to allow mmap for NOMMU, his case is also addressed in this
> series (PATCH 1/5 and PATCH 3/5).
>
> Thanks!
I have tested this v4 on my setup (stm32f4, no cache, no MPU) and unfortunately
it doesn't work with my drm/kms driver.
I haven't any errors but nothing is displayed unlike what I have when
using current dma-mapping
code.
I guess the issue is coming from dma-noop where __get_free_pages() is
used instead of alloc_pages()
in dma-mapping.
Since my hardware doesn't have cache or MPU (and so use dma-noop) I
haven't reserved specific memory region.
Buffer addresses and vma parameters look correct... What could I have
miss here ?
Benjamin
>
> [1] http://www.armlinux.org.uk/developer/patches/viewpatch.php?id=8633/1
>
> Vladimir Murzin (5):
> dma: Add simple dma_noop_mmap
> drivers: dma-coherent: Introduce default DMA pool
> ARM: NOMMU: Introduce dma operations for noMMU
> ARM: NOMMU: Set ARM_DMA_MEM_BUFFERABLE for M-class cpus
> ARM: dma-mapping: Remove traces of NOMMU code
>
> .../bindings/reserved-memory/reserved-memory.txt | 3 +
> arch/arm/include/asm/dma-mapping.h | 3 +-
> arch/arm/mm/Kconfig | 2 +-
> arch/arm/mm/Makefile | 5 +-
> arch/arm/mm/dma-mapping-nommu.c | 252 +++++++++++++++++++++
> arch/arm/mm/dma-mapping.c | 26 +--
> drivers/base/dma-coherent.c | 59 ++++-
> lib/dma-noop.c | 21 ++
> 8 files changed, 335 insertions(+), 36 deletions(-)
> create mode 100644 arch/arm/mm/dma-mapping-nommu.c
>
> --
> 2.0.0
>
--
Benjamin Gaignard
Graphic Study Group
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox