From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-la0-f41.google.com ([209.85.215.41]:35586 "EHLO mail-la0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755808AbbGCPBU (ORCPT ); Fri, 3 Jul 2015 11:01:20 -0400 Received: by lagh6 with SMTP id h6so88335872lag.2 for ; Fri, 03 Jul 2015 08:01:18 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20150629220929.GB1165@omega> <20150701082130.GA1834@omega> <20150701151653.GA969@omega> <20150702161239.GA1258@omega> Date: Fri, 3 Jul 2015 17:01:18 +0200 Message-ID: Subject: Re: ping6 doesn't use at86rf230 driver From: Baptiste Clenet Content-Type: text/plain; charset=UTF-8 Sender: linux-wpan-owner@vger.kernel.org List-ID: To: Alexander Aring Cc: Varka Bhadram , linux-wpan@vger.kernel.org 2015-07-03 15:37 GMT+02:00 Baptiste Clenet : > 2015-07-03 15:24 GMT+02:00 Baptiste Clenet : >> 2015-07-02 18:12 GMT+02:00 Alexander Aring : >>> On Thu, Jul 02, 2015 at 05:02:00PM +0200, Baptiste Clenet wrote: >>>> 2015-07-01 17:16 GMT+02:00 Alexander Aring : >>>> > Hi, >>>> > >>>> > On Wed, Jul 01, 2015 at 01:59:58PM +0200, Baptiste Clenet wrote: >>>> >> 2015-07-01 11:45 GMT+02:00 Baptiste Clenet : >>>> >> > 2015-07-01 11:22 GMT+02:00 Baptiste Clenet : >>>> >> >> >>>> >> >> 2015-07-01 10:21 GMT+02:00 Alexander Aring : >>>> >> >> > Hi, >>>> >> >> > >>>> >> >> > On Tue, Jun 30, 2015 at 11:12:36AM +0200, Baptiste Clenet wrote: >>>> >> >> > .... >>>> >> >> >> >>>> >> >> >> root@OpenWrt:/# dmesg | grep at86rf230 >>>> >> >> >> [ 94.820000] at86rf230 spi32766.1: Detected at86rf212 chip version 3 >>>> >> >> >> [ 94.830000] at86rf230 spi32766.1: unexcept state change from 0x00 >>>> >> >> >> to 0x08. Actual state: 0x00 >>>> >> >> >> >>>> >> >> >> It detects the chip but yes definitely, there is problem to read the state. >>>> >> >> >> Will check the pins >>>> >> >> >> >>>> >> >> > >>>> >> >> > if you have debugfs support and mounted it, then you could dump all >>>> >> >> > register settings by doing something similar like: >>>> >> >> > >>>> >> >> > cat /sys/kernel/debug/regmap/spi1.0/registers >>>> >> >> > >>>> >> >> > result would be some $REGISTER <-> $VALUE mapping. >>>> >> >> > >>>> >> >> > Note: >>>> >> >> > >>>> >> >> > One interface of the 802.15.4 phy should be up for this development method, >>>> >> >> > because the transceiver isn't in sleep mode then. >>>> >> >> > >>>> >> >> > - Alex >>>> >> >> >>>> >> >> The mapping: >>>> >> >> >>>> >> >> root@OpenWrt:/# cat /sys/kernel/debug/regmap/spi32766.1/registers >>>> >> >> 01: 16 >>>> > >>>> > this looks good, because 01 is a volatile register. Means it can be >>>> > changed during runtime by the transceiver and regmap _always_ get the >>>> > current register values when we send a get request. >>>> > >>>> > And it looks good, because you don't reading zeros on this register. >>>> > >>>> >> >> 02: f6 >>>> >> >> 03: 10 >>>> >> >> 04: 20 >>>> >> >> 05: 60 >>>> >> >> 06: 80 >>>> >> >> 07: 2c >>>> >> >> 08: 25 >>>> >> >> 09: 77 >>>> >> >> 0a: 17 >>>> >> >> 0b: a7 >>>> >> >> 0c: a4 >>>> >> >> 0d: 01 >>>> >> >> 0e: 08 >>>> >> >> 10: 44 >>>> >> >> 11: a2 >>>> >> >> 12: f0 >>>> >> >> 15: 00 >>>> >> >> 17: 00 >>>> >> >> 18: 50 >>>> >> >> 1a: 47 >>>> >> >> 1b: 54 >>>> >> >> 1c: 07 >>>> >> >> 1d: 03 >>>> >> >> 1e: 1f >>>> >> >> 1f: 00 >>>> >> >> 20: ff >>>> >> >> 21: ff >>>> >> >> 22: ef >>>> >> >> 23: be >>>> >> >> 24: 05 >>>> >> >> 25: 45 >>>> >> >> 26: 92 >>>> >> >> 27: 92 >>>> >> >> 28: 1e >>>> >> >> 29: 52 >>>> >> >> 2a: e4 >>>> >> >> 2b: d0 >>>> >> >> 2c: 38 >>>> >> >> 2d: 98 >>>> >> >> 2e: 42 >>>> >> >> 2f: 53 >>>> >> >> >>>> >> >> >>>> >> >> -- >>>> >> >> Baptiste >>>> >> > >>>> >> > >>>> >> > >>>> >> > I can set a different channel and see the difference in the regmap, >>>> >> > I'm definitely able to communicate with the transceiver. >>>> > >>>> > Don't trust change on registers which are not volatile. Regmap will do >>>> > some caching after the frist GET request of a register. If the value is >>>> > changed, regmap will not do a get request before again the first one. >>>> > The cached value will be used and then we do some bit magic on the >>>> > cached values and send a set register value command on the bus. >>>> > >>>> > This will reduce some traffic on the bus for configuration setting only, >>>> > which are done by regmap. >>>> > >>>> > When you want to look if the value for channel settings is really >>>> > changed, then simple add the "RG_PHY_CC_CCA" value to the volatile >>>> > function, see [1]. Otherwise the value is cached. >>>> > >>>> > btw: >>>> > For transmit/receive handling we use lowlevel spi_async calls on >>>> > registers which are volatile. >>>> > >>>> >> > I'm not sure about my interrupt pin definition in my dts. That might >>>> >> > be the problem. >>>> >> > >>>> >> > in palmbus >>>> >> > spi >>>> >> > at86rf212@0 { >>>> >> > compatible = "atmel,at86rf212"; >>>> >> > reg = <1>; >>>> >> > interrupt-parent = <&gpio0>; >>>> >> > interrupts = <15 1>; >>>> > >>>> > Please use "interrupts = <15 4>;" here, 4 indicates high-level triggered >>>> > interrupt and I experience on some systems deadlocks (on newer driver >>>> > versions you will get a warning about that). >>>> > >>>> > The reason is that we need to protect some irq resources and we do a >>>> > disable_irq and enable_irq path. See [0]. On _some_ architectures while >>>> > edge-triggered while irq is disabled, the irq won't fire after >>>> > enable_irq. In other hand high-level triggered irq will fire after >>>> > enable_irq. >>>> > >>>> >> > reset-gpio = <&gpio0 16 1>; >>>> >> > sleep-gpio = <&gpio0 17 1>; >>>> >> > spi-max-frequency = <1000000>; >>>> > >>>> > Maybe try there 4-5 Mhz? >>>> > >>>> >> > }; >>>> >> > >>>> >> > >>>> >> > and gpio >>>> >> > gpio@600 { >>>> >> > #address-cells = <1>; >>>> >> > #size-cells = <0>; >>>> >> > interrupt-parent = <&intc>; >>>> >> > interrupts = <6>; >>>> >> > >>>> >> > compatible = "mtk,mt7628-gpio", "mtk,mt7621-gpio"; >>>> >> > reg = <0x600 0x100>; >>>> >> > >>>> >> > gpio0: bank@0 { >>>> >> > reg = <0>; >>>> >> > ... >>>> >> > >>>> >> > >>>> >> > I define pin 15 as the interrupt pin here but how can I check it while >>>> >> > OpenWRT is running? >>>> > >>>> > I can only review the at86rf212 entry, don't know how to mux your pins >>>> > on your architecture correctly. >>>> > >>>> > You could try to make a "cat /proc/interrupts", this will show all >>>> > interrupts and check if the interrupt is increased, but the interrupt >>>> > should only increased by receiving and transmit complete. >>>> > >>>> > ... >>>> >> >>>> >> I'm wondering how regmap works. the at86rf230 write and read functions >>>> >> call regmap functions. I suppose regmap communicates with spi? >>>> >> >>>> >> Are values displayed by 'cat >>>> >> /sys/kernel/debug/regmap/spi32766.1/registers' read from the >>>> >> transceiver? (updated every time) or is it in the flash of my board >>>> >> and change one by one when it is required? >>>> >> >>>> >> When I change the channel, it obviously changes the regmap but how may >>>> >> I check that at86rf230 channel is really edited? (calling spi >>>> >> function) >>>> >> >>>> > >>>> > See my above comments about "how regmap works" there are registers which >>>> > are cached, but also some registers which can't be cached -> volatile >>>> > registers. For testing you could add the RG_PHY_CC_CCA register to the >>>> > volatile function. >>>> > >>>> > - Alex >>>> > >>>> > [0] http://lxr.free-electrons.com/source/drivers/net/ieee802154/at86rf230.c#L930 >>>> > [1] http://lxr.free-electrons.com/source/drivers/net/ieee802154/at86rf230.c#L422 >>>> >>>> Hi! >>>> >>>> Thanks a lot Alex for the clarification! Regmap is clear for me now :-) >>>> I've looked at every single exchange over SPI for the AT86RF230, I >>>> better understand the functioning and how your driver initializes it. >>>> (I've worked ont the 212B on another platform) >>>> >>> >>> The transceiver is after reset inside the RESET state, look "Extended >>> Operation Mode" inside at86rf212 datasheet. >>> >>> On probing for init the hw we going into TRX_OFF state. >>> >>>> Everything seems fine apart from the "at86rf230 spi32766.1: unexcept >>>> state change from 0x00 to 0x08. Actual state: 0x00" >>>> 0x01 (RG_TRX_STATUS) is volatile so the value in regmap is up to date. >>>> When setting up the transceiver, it goes to TRX_OFF which is the >>>> default state after starting. Why is it unexpected? >>>> >>> >>> On probing we need to "wait until the state change is done" while >>> hw_init. This is why we introduced the SYNCED state_change function. It >>> blocks while it is done. See [0]. >>> >>> This synced mechanism is build above the async state change. It does: >>> >>> 1. wait_for_completion_timeout which completes until somebody called a >>> "complete" >>> 2. in the complete handler of at86rf230_async_state_change we call the >>> complete -> this lets wait_for_completion_timeout unblock. >>> >>> This is the big difference between the async and synced framework of >>> state change. >>> >>> sync -> blocks until it's done. >>> async -> it does not block and have a complete handler. >>> >>> In some context of linux, of course in hard/soft irqs. We can't call >>> synced operations because it will run some scheduling and blocks the >>> function. You don't want that, if you do that you will get some "BUG - >>> scheduling while atomic". >>> >>> We have one callback "xmit_async" which is called in such context -> >>> soft irq. Or the isr routine -> hard irq. >>> >>> I hope until now it's all correct what I tell you. I am also not 100% >>> sure of this, but this is my point of view. >>> >>>> I don't really understand all the functions related to state change >>>> and async ... If you've got some time to explain me that and it will >>>> be maybe easier for me to debug this part and see why there is an >>>> unexpected change. >>>> >>> >>> For the state change calls we don't using the regmap framework, we using >>> the lowlevel spi_async calls. We use volatile registers there which are >>> not used by the regmap framework (except dumping the register values >>> over debugfs). >>> >>> If regmap dumping works for you maybe then spi_async calls doesn't work >>> for you? We can't also use regmap for everything, we need to load the >>> data into the frame buffer and regmap is only for register settings, not >>> huge payload of data. >>> >>> >>> I don't know why you reading zeros only on your spi bus, I would check >>> if the chipselect is right when calling spi_async. Did you changed >>> something according the chipselect handling? Again, note when we change >>> the state we don't using at86rf230_write/read/_*reg functions, we use >>> lowlevel spi calls. >>> >>> - Alex >>> >>> [0] http://lxr.free-electrons.com/source/drivers/net/ieee802154/at86rf230.c#L749 >> >> >> That was a very clear and complete answer, thanks! >> >> I've gone through the process of changing the states. Effectively >> spi_async() has got a problem (in comparaison to regmap call). I found >> some problems (they're maybe only dependent on my board (spi driver)). >> These errors happen at hw_init, here is the process: >> hw_init: >> at86rf230_sync_state_change(lp, STATE_FORCE_TRX_OFF); >> >> at86rf230_async_state_change(lp, &lp->state, state, >> at86rf230_sync_state_change_complete, false); >> >> at86rf230_async_read_reg(lp, RG_TRX_STATUS, ctx, >> at86rf230_async_state_change_start, irq_enable); >> spi_async(lp->spi, &ctx->msg); >> (->go to 'at86rf230_async_state_change_start' on callback) >> >> >> at86rf230_async_state_change_start >> if (trx_state == ctx->to_state) go to 'at86rf230_sync_state_change_complete' >> else { >> spi_async(lp->spi, &ctx->msg); >> (->go to 'at86rf230_async_state_delay' on callback) >> I see in [0] that this process have been changed .... I'm not sure I can easily use the last version (Bluetooth-next) on Linux 4.1.0, I see many changes on the last version :\ [0] http://git.kernel.org/cgit/linux/kernel/git/bluetooth/bluetooth-next.git/tree/drivers/net/ieee802154/at86rf230.c >> >> - in at86rf230_async_read_reg, ctx->trx.len = 2 so the spi driver >> receives 0x8100 instead of 0x81 to read TRX_STATUS which results to no >> readings (for me)! The transceiver returns 0 >> ---> I set ctx->trx.len to 1 and I receive 8 (TRX_OFF) which seems good. >> >> -- in at86rf230_async_state_change_start, > By the way, in at86rf230_async_state_change_start, const u8 trx_state > = buf[1] & TRX_STATE_MASK; but for my spi driver store the value in > buf[0] instead of buf[1], I needed to change it. >>we check if (trx_state == >> ctx->to_state), current state are: trx_state 8, ctx->to_state 3, Why >> are we checking if ctx->to_state 3? Because it's impossible to get 3 >> in TRX_STATUS, isn't it? So we should check for a 8 here? >> >> This is where I am now :-) >> >> >> -- >> Baptiste > > > > -- > Baptiste -- Baptiste