Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] mt76: fix build for MediaTek MT7610U USB wireless dongle
From: Kalle Valo @ 2018-08-09 15:13 UTC (permalink / raw)
  To: Valdis Kletnieks; +Cc: David S. Miller, netdev, linux-kernel, linux-wireless
In-Reply-To: <145751.1533759414@turing-police.cc.vt.edu>

Valdis Kletnieks <valdis.kletnieks@vt.edu> wrote:

> The mt76x0 driver requires the mt76 core driver to actually function.
> So add a 'select' to avoid embarrassing 'symbol unknown' errors
> when attempting to modprobe it in a module tree that doesn't
> include mt76.ko
> 
> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>

Patch applied to wireless-drivers-next.git, thanks.

1ec49a236565 mt76: fix build for MediaTek MT7610U USB wireless dongle

-- 
https://patchwork.kernel.org/patch/10560477/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* [PATCH net-next 0/7] s390/qeth: updates 2018-08-09
From: Julian Wiedmann @ 2018-08-09 12:47 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
	Stefan Raspl, Ursula Braun, Julian Wiedmann

Hi Dave,

one more set of patches for net-next. This is all sorts of cleanups and
more refactoring on the way to using netdev_priv. Please apply.

Thanks,
Julian


Gustavo A. R. Silva (1):
  s390/qeth: use true and false for boolean values

Julian Wiedmann (6):
  s390/qeth: extract helper for MPC protocol type
  s390/qeth: reduce hard-coded access to ccw channels
  s390/qeth: use qeth_setup_ccw() to set up all CCWs
  s390/qeth: do basic setup for data channel
  s390/qeth: clean up card initialization
  s390/qeth: don't restrict qeth_card to DMA memory

 drivers/s390/net/qeth_core.h      |   6 +-
 drivers/s390/net/qeth_core_main.c | 202 ++++++++++++++++----------------------
 drivers/s390/net/qeth_core_mpc.c  |  11 ---
 drivers/s390/net/qeth_core_mpc.h  |   4 -
 drivers/s390/net/qeth_l2_main.c   |  15 +--
 drivers/s390/net/qeth_l3_main.c   |   4 +-
 6 files changed, 98 insertions(+), 144 deletions(-)

-- 
2.16.4

^ permalink raw reply

* Re: [RFC net-next 00/15] net: A socket API for LoRa
From: Alexander Aring @ 2018-08-09 15:12 UTC (permalink / raw)
  To: Alan Cox
  Cc: Andreas Färber, Jian-Hong Pan, netdev, linux-arm-kernel,
	linux-kernel, Jiri Pirko, Marcel Holtmann, David S. Miller,
	Matthias Brugger, Janus Piwek, Michael Röder, Dollar Chen,
	Ken Yu, Konstantin Böhm, Jan Jongboom, Jon Ortego, contact,
	Ben Whitten, Brian Ray, lora
In-Reply-To: <20180809125939.39ac2cc9@alans-desktop>

Hi,

On Thu, Aug 09, 2018 at 12:59:39PM +0100, Alan Cox wrote:
> > Yes, and we are talking about that concrete sx1276 driver here, whose
> > chipset has a state machine that only allows either rx or tx and also
> > has standby and sleep modes with differing levels of data retention.
> 
> It's a hardware limit, it should never influence the protocol stack
> itself just the driver. Linux always tries to design to optimize the
> non-crappy case. In the long term that works out best because hardware
> improves and you don't want to be tied to an old limit.
> 
> > > (Some ancient ethernet cards do this btw.. they can't listen and transmit
> > > at the same time)  
> > 
> > So when do they start receiving?
> 
> When they are not transmitting. The transmit path switches modes and when
> the frame send is done it goes back to receiving. As old ethernet was
> also half duplex that worked.
> 

We do the same at some IEEE 802.15.4 transceivers. A transceiver has
_one_ framebuffer only for tx and rx. Another one has two framebuffer
separated tx and rx, but is half duplex.

There is a little performance tweak in separated framebuffers that you
can fill up the tx framebuffer while the transceiver receives the frame
(completely independet from any bus communicaten/linux handling).

> > The issue here was that my original description, which you appear to
> > have cut, suggested a continuous listen mode, interrupted by transmit.
> 
> I don't think I cut it but if so I didn't mean to and your approach is
> the one I agree with.
> 
...

> 
> There is a heirarchy. Let me us IP for an example
> 
> (historically it was SOCK_PACKET nowdays PF_PACKET - the layering got
> sorted better)
> 
> PF_PACKET SOCK_RAW ETH_P_ALL		
> 
> Everything on that device minus some things like hardware pre-ambles
> 
> PF_PACKET SOCK_RAW ETH_P_SOMETHING
> 
> Everything on that device that has the underlying protocol (and the
> protocol might not be in the packet but a property of the interface
> because it only does that format - simple example SLIP is IP packets over
> a serial link a SLIP interface is IP, not because there is anything
> saying it is but because that is *all* it can be)
> 
> You get the two above for free. PF_PACKET is built into the stack so
> providing you label packets with the ETH_P_xxx you have for Lora, you can
> use PF_PACKET interfaces to dump them and write raw packets at the kernel
> layer.
> 

In 802.15.4:

We recommend nowadays to use PF_PACKET raw sockets for construct L2
frames in userspace. We use that mostly to connect some user space
stacks to make some interop testing without hardware being involved.

For DGRAM sockets, due lack of UAPI limitations of sockaddr_t we
have our own implementation. DGRAM in PF_PACKET use some limitated
feature to "just send something" to an unique address scheme... but some
users need more access because 802.15.4 address scheme is complex.

> PF_INET SOCK_RAW

I think LoRa should look into the 6lowpan subsystem of the Linux kernel
for that. 6lowpan is known as some "IPv6-over-foo" adaptations. Mostly
you just need to implement some mapping from L2 address to SLAAC IPv6 address
pattern only. I see there is a draft at 6lo wg [0] for that which is
expired 2016 (But I would not care about that).

At the end you have a master IP capable interface and your slave is your L2
interface. The 6lowpan interface is a RAW IP interface and do a protocol
translation in the background. On L2 interface you will see L2 + 6LoWPAN
+ $IP_UPPER_LAYER.

Current benefits are more compression, but there exists also some ndisc
optimizations for low power networks which we don't support right now.

- Alex

[0] https://www.ietf.org/archive/id/draft-vilajosana-6lpwa-lora-hc-01.txt

^ permalink raw reply

* Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper
From: Greg KH @ 2018-08-09 12:47 UTC (permalink / raw)
  To: maowenan
  Cc: David Woodhouse, davem, edumazet, juha-matti.tilli, ycheng,
	soheil, netdev, eric.dumazet, dwmw2, jdw
In-Reply-To: <20e03726-c053-67a3-505a-7110be0f5d34@huawei.com>

On Thu, Aug 09, 2018 at 08:37:13PM +0800, maowenan wrote:
> 
> 
> On 2018/8/7 21:22, Greg KH wrote:
> > On Sat, Aug 04, 2018 at 10:10:00AM +0100, David Woodhouse wrote:
> >> From: Eric Dumazet <edumazet@google.com>
> >>
> >> commit 58152ecbbcc6a0ce7fddd5bf5f6ee535834ece0c upstream.
> >>
> >> In case skb in out_or_order_queue is the result of
> >> multiple skbs coalescing, we would like to get a proper gso_segs
> >> counter tracking, so that future tcp_drop() can report an accurate
> >> number.
> >>
> >> I chose to not implement this tracking for skbs in receive queue,
> >> since they are not dropped, unless socket is disconnected.
> >>
> >> Signed-off-by: Eric Dumazet <edumazet@google.com>
> >> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> >> Acked-by: Yuchung Cheng <ycheng@google.com>
> >> Signed-off-by: David S. Miller <davem@davemloft.net>
> >> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> >> ---
> >>  net/ipv4/tcp_input.c | 23 +++++++++++++++++++++--
> >>  1 file changed, 21 insertions(+), 2 deletions(-)
> > 
> > Now applied, thanks,
> > 
> > greg k-h
> > 
> > .
> > 
> 
> Hello,
> 
> There are two patches in stable branch linux-4.4, but I have tested with below patches, and found that the cpu usage was very high.
> dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
> 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
> 
> test results:
> with fix patch: 78.2%   ksoftirqd
> no fix patch:   90%     ksoftirqd
> 
> there is %0 when no attack packets.
> 
> so please help verify that fixed patches are enough in linux-stable 4.4.
> 

I do not know, I am not a network developer.  Please try to reproduce
the same thing on a newer kernel release and see if the result is the
same or not.  If you can find a change that I missed, please let me know
and I will be glad to apply it.

thnaks,

greg k-h

^ permalink raw reply

* Re: [Query]: DSA Understanding
From: Lad, Prabhakar @ 2018-08-09 12:45 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20180809120159.GA16359@lunn.ch>

On Thu, Aug 9, 2018 at 1:02 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Aug 09, 2018 at 12:31:31PM +0100, Lad, Prabhakar wrote:
> > Hi Andrew,
> >
> > On Thu, Aug 2, 2018 at 5:05 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > I dont see any Reply's on the PC with tcpdump on PC
> > >
> > > So try ethool -S on the PC. Any packets dropped because of errors?
> > >
> > I dont see any drops/errors on the PC, following is the dump from PC:
> >
> > sudo ethtool -S enx00e04c68c229
> > [sudo] password for prabhakar:
> > NIC statistics:
> >      tx_packets: 1659
> >      rx_packets: 485
> >      tx_errors: 0
> >      rx_errors: 0
> >      rx_missed: 0
> >      align_errors: 0
> >      tx_single_collisions: 0
> >      tx_multi_collisions: 0
> >      rx_unicast: 18
> >      rx_broadcast: 295
> >      rx_multicast: 172
> >      tx_aborted: 0
> >      tx_underrun: 0
>
> So there are received packets at the PC. Not many unicast, mostly
> broadcast, which fits with ARP. What does wireshark tell you about
> these received packets? Are they ARP replies? Are they something else?
> If they are ARP replies, why are they being ignored?  I don't know if
> tshark will show you CRC problems. Wireshark does, when you unfold a
> packet, and look at the fields in detail.
>
> > Seems like the packet is not being transmitted from the switch at all
> > ? (as ping from switch lan4 to PC fails)
>
> I don't think you can make that conclusion yet. The PC is receiving
> something, rx_packets=485. What are those packets?
>
The received packets captured on the PC are MDNS and DHPC, these MDNS
are causing the rx
packet counter go up:

685    682.956266963    0.0.0.0    255.255.255.255    DHCP    422
DHCP Discover - Transaction ID 0x9d832fac
550    555.543322200    0.0.0.0    255.255.255.255    DHCP    422
DHCP Discover - Transaction ID 0x9d832fac
572    576.738682378    fe80::2f12:3d45:7cca:57fa    ff02::fb    MDNS
  136    Standard query 0x0000 AAAA VB4-SN00000000.local, "QM"
question SRV VB4-SN00000000._sftp-ssh._tcp.local, "QM" question
630    630.706578680    169.254.126.126    224.0.0.251    MDNS    168
  Standard query response 0x0000 AAAA, cache flush
fe80::c6f3:12ff:fe08:fe7f SRV, cache flush 0 0 22 VB4-SN00000000.local
A, cache flush 169.254.126.126
732    728.982449369    169.254.78.251    224.0.0.251    MDNS    122
 Standard query 0x0000 AAAA VB4-SN00000000.local, "QM" question A
VB4-SN00000000.local, "QM" question SRV
VB4-SN00000000._sftp-ssh._tcp.local, "QM" question
733    728.983534948    169.254.126.126    224.0.0.251    MDNS    168
  Standard query response 0x0000 SRV, cache flush 0 0 22
VB4-SN00000000.local AAAA, cache flush fe80::c6f3:12ff:fe08:fe7f A,
cache flush 169.254.126.126

> Look at the statistics along the chain, from the target to the PC.
> Look at the master device, lan4 and the PC. You should see about one
> packet per second transmitted on the master device. One packet per
> second transmitted on lan4, and one packet per second received on the
> PC. Where does this break down.
>
I don’t see any packets reaching the PC for the ping request. I can see the
RX and TX on the switch for lan4 increasing every second. seems like the
switch itself is consuming it and not forwarding(but then lan4 TX
shouldn’t have incremented ?).

any thoughts where I could focusing on the switch or cpsw ?

Cheers,
--Prabhakar Lad

^ permalink raw reply

* Re: [PATCH wireless-drivers] mt76x0: Remove VLA usage
From: Kalle Valo @ 2018-08-09 15:09 UTC (permalink / raw)
  To: Kees Cook
  Cc: Stanislaw Gruszka, David S. Miller, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <20180807225040.GA2164@beast>

Kees Cook <keescook@chromium.org> wrote:

> Even with "const" variables, the compiler will generate warnings about
> VLA usage. In the quest to remove all VLAs from the kernel[1], this uses
> a #define instead of a const to do the array sizing.
> 
> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
> 
> Fixes: e87b5039511a ("mt76x0: eeprom files")
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>

Patch applied to wireless-drivers-next.git, thanks.

17ad18fd12a3 mt76x0: Remove VLA usage

-- 
https://patchwork.kernel.org/patch/10559297/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* [PATCH net-next] net: dp83640: Mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2018-08-09 15:08 UTC (permalink / raw)
  To: Richard Cochran, Andrew Lunn, Florian Fainelli, David S. Miller
  Cc: netdev, linux-kernel, Gustavo A. R. Silva

In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Notice that in this particular case, I replaced the code comment at the
top of the switch statement with a proper "fall through" annotation for
each case, which is what GCC is expecting to find.

Addresses-Coverity-ID: 1056542 ("Missing break in switch")
Addresses-Coverity-ID: 1339579 ("Missing break in switch")
Addresses-Coverity-ID: 1369526 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/phy/dp83640.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 79e9b10..29aa8d7 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -757,13 +757,16 @@ static int decode_evnt(struct dp83640_private *dp83640,
 
 	phy_txts = data;
 
-	switch (words) { /* fall through in every case */
+	switch (words) {
 	case 3:
 		dp83640->edata.sec_hi = phy_txts->sec_hi;
+		/* fall through */
 	case 2:
 		dp83640->edata.sec_lo = phy_txts->sec_lo;
+		/* fall through */
 	case 1:
 		dp83640->edata.ns_hi = phy_txts->ns_hi;
+		/* fall through */
 	case 0:
 		dp83640->edata.ns_lo = phy_txts->ns_lo;
 	}
-- 
2.7.4

^ permalink raw reply related

* Re: [RFC net-next 00/15] net: A socket API for LoRa
From: Jian-Hong Pan @ 2018-08-09 15:02 UTC (permalink / raw)
  To: Alan Cox
  Cc: Michal Kubeček, Konstantin Böhm, shess, pieter.robyns,
	linux-kernel@vger.kernel.org>, , Xue Liu, Ken Yu,
	Michael Röder, Stefan Schmidt, Rob Herring, lora,
	Alexander Graf, Jan Jongboom, Janus Piwek, Jon Ortego, devicetree,
	Jiri Pirko, Hasnain Virk, Daniele Comel, Marcel Holtmann,
	linux-spi, Mark Brown, Dollar Chen, Brian Ray
In-Reply-To: <20180809125939.39ac2cc9@alans-desktop>

Alan Cox <gnomes@lxorguk.ukuu.org.uk> 於 2018年8月9日 週四 下午7:59寫道:
>
> > Yes, and we are talking about that concrete sx1276 driver here, whose
> > chipset has a state machine that only allows either rx or tx and also
> > has standby and sleep modes with differing levels of data retention.
>
> It's a hardware limit, it should never influence the protocol stack
> itself just the driver. Linux always tries to design to optimize the
> non-crappy case. In the long term that works out best because hardware
> improves and you don't want to be tied to an old limit.
>
> > > (Some ancient ethernet cards do this btw.. they can't listen and transmit
> > > at the same time)
> >
> > So when do they start receiving?
>
> When they are not transmitting. The transmit path switches modes and when
> the frame send is done it goes back to receiving. As old ethernet was
> also half duplex that worked.
>
> > The issue here was that my original description, which you appear to
> > have cut, suggested a continuous listen mode, interrupted by transmit.
>
> I don't think I cut it but if so I didn't mean to and your approach is
> the one I agree with.
>
> > Jian-Hong didn't like that, with reference to the LoRaWAN spec that
> > supposedly asks for only being in receive mode when expecting a message,
> > likely to save on battery. So the question is, could we cleanly
> > implement receiving only when the user asks us to, or is that a no-go?
>
> Why would you do so ? You can't run Linux on a tiny little
> micro-controller where that would matter. Sure it makes sense for some
> tiny spec of embedded silicon buried in a sensor - but not a Linux box.
>
> Now you might power it down when the interface is down, or when there is
> nobody using that interface but that's really more about long term idle
> power.

Except saving power, mitigating the wireless signal conflict on the
air is one of the reasons.

According to the LoRaWAN spec. defined by LoRa Alliance, all LoRaWAN
end devices must implement class A features, which is also the most
general and simplest.
1. The transmission slot scheduled by the end-device is based on its
own communication needs with a small variation based on a random time
basis (ALOHA-type of protocol).
2. Only require downlink communication from the server shortly after
the end-device has sent an uplink transmission. Downlink
communications from the server at any other time will have to wait
until the next scheduled uplink.
3. Each end-device‘s uplink transmission is followed by two short
downlink receive windows.

End device transmits an uplink message finished
   ->
End device goes to sleep/idle/stop
   ->
End device opens RX1 window.  This is the time for the downlink message.
   ->
End device goes to sleep/idle/stop
   ->
End device opens RX2 window.  This is another time for the downlink message.
   ->
End device goes to sleep/idle/stop

The time between end device transmit an uplink message finished and
end device opens RX1 window is RX delay #1.
The time between end device transmit an uplink message finished and
end device opens RX2 window is RX delay #2.
If end device gets the meaningful downlink message in RX1 window, it
will not open RX2 window.
The time of RX delay #1 and 2 are defined in LoRaWAN Regional Parameters.

The sleep/idle/stop mitigate the unconcerned RF signals or messages.

> > bands and 2.) duty-cycle limits for some of those bands. No maintainer
> > commented on that so far. Thus I am working in tiny steps on providing
> > netlink-layer commands in nllora that can dispatch the individual radio
> > settings to drivers, which then upper layers can instrument as needed.
>
> Sounds right to me.
> >
> > And making my very first steps with netlink here, it appeared as if each
> > technology has its own enums of commands and attributes, so I don't see
> > how to reuse anything from Wifi here apart from some design inspiration.
>
> That seems reasonable - you aren't likely to want to manage them with the
> same tool.
>
> > > That's a hardware question. Imagine a software defined radio. If your
> > > limitation wouldn't exist in a pure software defined radio then it's
> > > almost certainly a device level detal.
> >
> > An SDR would not be using this sx1276 device driver, I imagine.
> >
> > In fact I would expect an SDR device not to be in drivers/net/lora/ at
> > all but to live in drivers/net/sdr/ and to consume ETH_P_LORA etc. skbs
> > and just do the right thing for them depending on their type...
>
> The point I was trying to make was that if you want to decide whether
> something is driver level or protocol level ask 'is this something you
> can't do even with an SDR'. Some things are protocol properties that no
> fancy hardware will change. Others are hardware limits, in which case you
> want them driver level - because at some point the hardware will get
> better.
>
> > > If you've got something listening to data but without the structure
> > > needed to identify multiple listeners and split out the data meaningfully
> > > to those listeners according to parts of the packet then you've got no
> > > reason to make it a protocol just use SOCK_PACKET and if need be BPF.
> >
> > Sorry, that doesn't parse for me. SOCK_PACKET must be a protocol on some
> > PF_ protocol family, no? Are you suggesting I use SOCK_PACKET instead of
> > SOCK_DGRAM in what is now net/lora/dgram.c? Or are you saying there's
> > some generic implementation that we can reuse and scratch mine?
>
> There is a heirarchy. Let me us IP for an example
>
> (historically it was SOCK_PACKET nowdays PF_PACKET - the layering got
> sorted better)
>
> PF_PACKET SOCK_RAW ETH_P_ALL
>
> Everything on that device minus some things like hardware pre-ambles
>
> PF_PACKET SOCK_RAW ETH_P_SOMETHING
>
> Everything on that device that has the underlying protocol (and the
> protocol might not be in the packet but a property of the interface
> because it only does that format - simple example SLIP is IP packets over
> a serial link a SLIP interface is IP, not because there is anything
> saying it is but because that is *all* it can be)
>
> You get the two above for free. PF_PACKET is built into the stack so
> providing you label packets with the ETH_P_xxx you have for Lora, you can
> use PF_PACKET interfaces to dump them and write raw packets at the kernel
> layer.
>
> PF_INET SOCK_RAW
>
> Split the messages by protocol number in IP between multiple
> listeners/writers
>
> PF_INET SOCK_UDP / TCP etc
>
> Split the messages by port numbers in the higher level protocol
>
>
> For PF_LORA these would map to whatever goes on at the LORA protocol
> level and divide LORA messages up between multiple processes on the
> Linux system that are interested in some of the messages.
>
>
> > > The reason we have a socket layer not /dev/ethernet0 is that it's
> > > meaningful to divide messages up into flows, and to partition those flows
> > > securely amongst multiple consumers/generators.
> >
> > For me the distinction is that a /dev/whatever0 would seem more suited
> > for a stream of data to read/write, whereas sockets give us a bounded
> > skb for packets at device driver level.
>
> You could equally do that in a simple character device *if* you didn't
> need to split messages up and share between users. Some protocol stacks
> actually do that and then sort it out in user space, either because they
> are really obscure or they are incredibly complicated and broken so want
> to be out of kernel 8)
>
> > These PHYs all broadcast something over the antenna when sending, with
> > any addressing of listeners or senders being optional and MAC-specific,
> > apart from the LoRa/FSK SyncWord as well as the various frequency etc.
> > settings that determine what the receiver listens for.
> >
> > None of these PHYs define any mechanism like EtherType through which to
> > identify upper-layer protocols.
> >
> > So in a way, listening is always in a promiscuous mode, and I guess we
> > would need to try to parse each incoming packet as e.g. a LoRaWAN packet
> > and just give up if it's too short or checksums don't match. Only at the
> > layer of LoRaWAN and competing proprietary or custom protocols can we
> > split received packets out to individual listeners.
>
> My vote would be in that case that you either
>
> 1. Set the protocol type on the interface assuming you don't mix and
> match (and if it's relying on random bits not looking like other packets
> then it sounds a complete mess at the moment - but yeah its new tech)
>
> 2. You pass everything up to some magical agent which somehow splits them
> up and labels them ETH_P_LORA / ETH_P_FOO etc
>
> 3. You do what ethernet does (which admittedly is *way* simpler for
> ethernet) and you have a library routine you can pass an skbuff in the
> driver itself which figures out wtf to label the packet. Look how
> eth_type_trans() is used. Drivers then just do
>
>            skb->protocol = xxx_type_trans(skb, dev);
>
> and the basic labelling gets done and any header pulls (you probably won't
> have any given you don't have anything wrapping LORA), and
> multicast/broadcast labelling - again meaningless I suspect.
>
> #3 Is probably the nicest because you update it all in one place as
> standards change and the market hopefully conslidates and develops some
> kind of sane packet formats. It's also effectively covering #1 and it's
> easy to start with because an initial implementation can just do 'return
> htons(ETH_P_LORA)'.
>
> >
> > Does that give us any further clues for the design discussion here?
> >
> I think so yes
>
> How do you plan to deal with routing if you've got multiple devices ?

For LoRaWAN, it is a star topology.

One end device and one LoRaWAN gateway:
An end device sends a uplink message to the LoRaWAN gateway directly.
A LoRaWAN gateway receives the uplink message, then transfers to the
LoRaWAN network server.
The network server sends a downlink message to the end device via the
transfer of the LoRaWAN gateway.

Multiple end devices and multiple LoRaWAN gateway:
Multiple end devices can send uplink messages to multiple LoRaWAN
gateways directly.  Gateways receive the the uplink messages, then
transfer to the LoRaWAN network server.
The LoRaWAN network server compares the uplink messages' content to
filter out the duplicated messages.
The LoRaWAN network server sends a downlink message to the end device
via the transfer of only one chosen LoRaWAN gateway.

So, an end device does not talk to other end devices directly.
And, of course, the links between LoRaWAN gateway and the network
server could be any network technology, only if they can communicate
with each other fast enough.

Regards,
Jian-Hong Pan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper
From: maowenan @ 2018-08-09 12:37 UTC (permalink / raw)
  To: Greg KH, David Woodhouse
  Cc: davem, edumazet, juha-matti.tilli, ycheng, soheil, netdev,
	eric.dumazet, dwmw2, jdw
In-Reply-To: <20180807132256.GH1904@kroah.com>



On 2018/8/7 21:22, Greg KH wrote:
> On Sat, Aug 04, 2018 at 10:10:00AM +0100, David Woodhouse wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> commit 58152ecbbcc6a0ce7fddd5bf5f6ee535834ece0c upstream.
>>
>> In case skb in out_or_order_queue is the result of
>> multiple skbs coalescing, we would like to get a proper gso_segs
>> counter tracking, so that future tcp_drop() can report an accurate
>> number.
>>
>> I chose to not implement this tracking for skbs in receive queue,
>> since they are not dropped, unless socket is disconnected.
>>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
>> Acked-by: Yuchung Cheng <ycheng@google.com>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
>> ---
>>  net/ipv4/tcp_input.c | 23 +++++++++++++++++++++--
>>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> Now applied, thanks,
> 
> greg k-h
> 
> .
> 

Hello,

There are two patches in stable branch linux-4.4, but I have tested with below patches, and found that the cpu usage was very high.
dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible

test results:
with fix patch: 78.2%   ksoftirqd
no fix patch:   90%     ksoftirqd

there is %0 when no attack packets.

so please help verify that fixed patches are enough in linux-stable 4.4.

^ permalink raw reply

* [PATCH lora-next v2 8/8] net: lora: sx1301: convert driver over to regmap reads and writes
From: Ben Whitten @ 2018-08-09 12:33 UTC (permalink / raw)
  To: afaerber; +Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten
In-Reply-To: <1533818018-29005-1-git-send-email-ben.whitten@lairdtech.com>

The reads and writes are replaced with regmap versions and unneeded
functions, variable, and defines removed.

Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
---
 drivers/net/lora/sx1301.c | 204 +++++++++++++++-------------------------------
 drivers/net/lora/sx1301.h |  30 +++++++
 2 files changed, 95 insertions(+), 139 deletions(-)

diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 766df06..4db5a43 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -24,29 +24,6 @@
 
 #include "sx1301.h"
 
-#define REG_PAGE_RESET			0
-#define REG_MCU_PROM_ADDR		9
-#define REG_MCU_PROM_DATA		10
-#define REG_GPIO_SELECT_INPUT		27
-#define REG_GPIO_SELECT_OUTPUT		28
-#define REG_GPIO_MODE			29
-#define REG_MCU_AGC_STATUS		32
-#define REG_0_RADIO_SELECT		35
-#define REG_0_MCU			106
-#define REG_2_SPI_RADIO_A_DATA		33
-#define REG_2_SPI_RADIO_A_DATA_READBACK	34
-#define REG_2_SPI_RADIO_A_ADDR		35
-#define REG_2_SPI_RADIO_A_CS		37
-#define REG_2_SPI_RADIO_B_DATA		38
-#define REG_2_SPI_RADIO_B_DATA_READBACK	39
-#define REG_2_SPI_RADIO_B_ADDR		40
-#define REG_2_SPI_RADIO_B_CS		42
-#define REG_2_DBG_ARB_MCU_RAM_DATA	64
-#define REG_2_DBG_AGC_MCU_RAM_DATA	65
-#define REG_2_DBG_ARB_MCU_RAM_ADDR	80
-#define REG_2_DBG_AGC_MCU_RAM_ADDR	81
-#define REG_EMERGENCY_FORCE		127
-
 #define REG_PAGE_RESET_SOFT_RESET	BIT(7)
 
 #define REG_16_GLOBAL_EN		BIT(3)
@@ -99,8 +76,7 @@ static struct regmap_config sx1301_regmap_config = {
 
 struct spi_sx1301 {
 	struct spi_device *parent;
-	u8 page;
-	u8 regs;
+	unsigned int regs;
 };
 
 struct sx1301_priv {
@@ -108,7 +84,6 @@ struct sx1301_priv {
 	struct spi_device	*spi;
 	struct lora_priv lora;
 	struct gpio_desc *rst_gpio;
-	u8 cur_page;
 	struct spi_controller *radio_a_ctrl, *radio_b_ctrl;
 	struct regmap		*regmap;
 };
@@ -140,50 +115,9 @@ static int sx1301_write(struct sx1301_priv *priv, u8 reg, u8 val)
 	return sx1301_write_burst(priv, reg, &val, 1);
 }
 
-static int sx1301_page_switch(struct sx1301_priv *priv, u8 page)
-{
-	int ret;
-
-	if (priv->cur_page == page)
-		return 0;
-
-	dev_dbg(priv->dev, "switching to page %u\n", (unsigned)page);
-	ret = sx1301_write(priv, REG_PAGE_RESET, page & 0x3);
-	if (ret) {
-		dev_err(priv->dev, "switching to page %u failed\n", (unsigned)page);
-		return ret;
-	}
-
-	priv->cur_page = page;
-
-	return 0;
-}
-
-static int sx1301_page_read(struct sx1301_priv *priv, u8 page, u8 reg, u8 *val)
-{
-	int ret;
-
-	ret = sx1301_page_switch(priv, page);
-	if (ret)
-		return ret;
-
-	return sx1301_read(priv, reg, val);
-}
-
-static int sx1301_page_write(struct sx1301_priv *priv, u8 page, u8 reg, u8 val)
-{
-	int ret;
-
-	ret = sx1301_page_switch(priv, page);
-	if (ret)
-		return ret;
-
-	return sx1301_write(priv, reg, val);
-}
-
 static int sx1301_soft_reset(struct sx1301_priv *priv)
 {
-	return sx1301_write(priv, REG_PAGE_RESET, REG_PAGE_RESET_SOFT_RESET);
+	return regmap_write(priv->regmap, SX1301_PAGE, REG_PAGE_RESET_SOFT_RESET);
 }
 
 #define REG_RADIO_X_DATA		0
@@ -195,12 +129,12 @@ static int sx1301_radio_set_cs(struct spi_controller *ctrl, bool enable)
 {
 	struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
 	struct sx1301_priv *priv = spi_get_drvdata(ssx->parent);
-	u8 cs;
+	unsigned int cs;
 	int ret;
 
 	dev_dbg(&ctrl->dev, "setting CS to %s\n", enable ? "1" : "0");
 
-	ret = sx1301_page_read(priv, ssx->page, ssx->regs + REG_RADIO_X_CS, &cs);
+	ret = regmap_read(priv->regmap, ssx->regs + REG_RADIO_X_CS, &cs);
 	if (ret) {
 		dev_warn(&ctrl->dev, "failed to read CS (%d)\n", ret);
 		cs = 0;
@@ -211,7 +145,7 @@ static int sx1301_radio_set_cs(struct spi_controller *ctrl, bool enable)
 	else
 		cs &= ~BIT(0);
 
-	ret = sx1301_page_write(priv, ssx->page, ssx->regs + REG_RADIO_X_CS, cs);
+	ret = regmap_write(priv->regmap, ssx->regs + REG_RADIO_X_CS, cs);
 	if (ret) {
 		dev_err(&ctrl->dev, "failed to write CS (%d)\n", ret);
 		return ret;
@@ -235,8 +169,8 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
 {
 	struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
 	struct sx1301_priv *priv = spi_get_drvdata(ssx->parent);
-	const u8 *tx_buf = xfr->tx_buf;
-	u8 *rx_buf = xfr->rx_buf;
+	const unsigned int *tx_buf = xfr->tx_buf;
+	unsigned int *rx_buf = xfr->rx_buf;
 	int ret;
 
 	if (xfr->len == 0 || xfr->len > 3)
@@ -245,13 +179,13 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
 	dev_dbg(&spi->dev, "transferring one (%u)\n", xfr->len);
 
 	if (tx_buf) {
-		ret = sx1301_page_write(priv, ssx->page, ssx->regs + REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
+		ret = regmap_write(priv->regmap, ssx->regs + REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
 		if (ret) {
 			dev_err(&spi->dev, "SPI radio address write failed\n");
 			return ret;
 		}
 
-		ret = sx1301_page_write(priv, ssx->page, ssx->regs + REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
+		ret = regmap_write(priv->regmap, ssx->regs + REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
 		if (ret) {
 			dev_err(&spi->dev, "SPI radio data write failed\n");
 			return ret;
@@ -271,7 +205,7 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
 	}
 
 	if (rx_buf) {
-		ret = sx1301_page_read(priv, ssx->page, ssx->regs + REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
+		ret = regmap_read(priv->regmap, ssx->regs + REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
 		if (ret) {
 			dev_err(&spi->dev, "SPI radio data read failed\n");
 			return ret;
@@ -281,17 +215,17 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
 	return 0;
 }
 
-static int sx1301_agc_ram_read(struct sx1301_priv *priv, u8 addr, u8 *val)
+static int sx1301_agc_ram_read(struct sx1301_priv *priv, u8 addr, unsigned int *val)
 {
 	int ret;
 
-	ret = sx1301_page_write(priv, 2, REG_2_DBG_AGC_MCU_RAM_ADDR, addr);
+	ret = regmap_write(priv->regmap, SX1301_DBG_AGC_MCU_RAM_ADDR, addr);
 	if (ret) {
 		dev_err(priv->dev, "AGC RAM addr write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(priv, 2, REG_2_DBG_AGC_MCU_RAM_DATA, val);
+	ret = regmap_read(priv->regmap, SX1301_DBG_AGC_MCU_RAM_DATA, val);
 	if (ret) {
 		dev_err(priv->dev, "AGC RAM data read failed\n");
 		return ret;
@@ -300,17 +234,17 @@ static int sx1301_agc_ram_read(struct sx1301_priv *priv, u8 addr, u8 *val)
 	return 0;
 }
 
-static int sx1301_arb_ram_read(struct sx1301_priv *priv, u8 addr, u8 *val)
+static int sx1301_arb_ram_read(struct sx1301_priv *priv, u8 addr, unsigned int *val)
 {
 	int ret;
 
-	ret = sx1301_page_write(priv, 2, REG_2_DBG_ARB_MCU_RAM_ADDR, addr);
+	ret = regmap_write(priv->regmap, SX1301_DBG_ARB_MCU_RAM_ADDR, addr);
 	if (ret) {
 		dev_err(priv->dev, "ARB RAM addr write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(priv, 2, REG_2_DBG_ARB_MCU_RAM_DATA, val);
+	ret = regmap_read(priv->regmap, SX1301_DBG_ARB_MCU_RAM_DATA, val);
 	if (ret) {
 		dev_err(priv->dev, "ARB RAM data read failed\n");
 		return ret;
@@ -322,7 +256,8 @@ static int sx1301_arb_ram_read(struct sx1301_priv *priv, u8 addr, u8 *val)
 static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct firmware *fw)
 {
 	u8 *buf;
-	u8 val, rst, select_mux;
+	u8  rst, select_mux;
+	unsigned int val;
 	int ret;
 
 	if (fw->size != SX1301_MCU_FW_BYTE) {
@@ -343,7 +278,7 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 		return -EINVAL;
 	}
 
-	ret = sx1301_page_read(priv, 0, REG_0_MCU, &val);
+	ret = regmap_read(priv->regmap, SX1301_MCU_CTRL, &val);
 	if (ret) {
 		dev_err(priv->dev, "MCU read failed\n");
 		return ret;
@@ -352,25 +287,25 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 	val |= rst;
 	val &= ~select_mux;
 
-	ret = sx1301_page_write(priv, 0, REG_0_MCU, val);
+	ret = regmap_write(priv->regmap, SX1301_MCU_CTRL, val);
 	if (ret) {
 		dev_err(priv->dev, "MCU reset / select mux write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_write(priv, REG_MCU_PROM_ADDR, 0);
+	ret = regmap_write(priv->regmap, SX1301_MPA, 0);
 	if (ret) {
 		dev_err(priv->dev, "MCU prom addr write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_write_burst(priv, REG_MCU_PROM_DATA, fw->data, fw->size);
+	ret = sx1301_write_burst(priv, SX1301_MPD, fw->data, fw->size);
 	if (ret) {
 		dev_err(priv->dev, "MCU prom data write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(priv, REG_MCU_PROM_DATA, &val);
+	ret = regmap_read(priv->regmap, SX1301_MPD, &val);
 	if (ret) {
 		dev_err(priv->dev, "MCU prom data dummy read failed\n");
 		return ret;
@@ -380,7 +315,7 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 	if (!buf)
 		return -ENOMEM;
 
-	ret = sx1301_read_burst(priv, REG_MCU_PROM_DATA, buf, fw->size);
+	ret = sx1301_read_burst(priv, SX1301_MPD, buf, fw->size);
 	if (ret) {
 		dev_err(priv->dev, "MCU prom data read failed\n");
 		kfree(buf);
@@ -395,7 +330,7 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 
 	kfree(buf);
 
-	ret = sx1301_page_read(priv, 0, REG_0_MCU, &val);
+	ret = regmap_read(priv->regmap, SX1301_MCU_CTRL, &val);
 	if (ret) {
 		dev_err(priv->dev, "MCU read (1) failed\n");
 		return ret;
@@ -403,7 +338,7 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 
 	val |= select_mux;
 
-	ret = sx1301_page_write(priv, 0, REG_0_MCU, val);
+	ret = regmap_write(priv->regmap, SX1301_MCU_CTRL, val);
 	if (ret) {
 		dev_err(priv->dev, "MCU reset / select mux write (1) failed\n");
 		return ret;
@@ -415,7 +350,7 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 {
 	const struct firmware *fw;
-	u8 val;
+	unsigned int val;
 	int ret;
 
 	ret = request_firmware(&fw, "sx1301_agc_calibration.bin", priv->dev);
@@ -431,7 +366,7 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 		return ret;
 	}
 
-	ret = sx1301_page_read(priv, 0, 105, &val);
+	ret = regmap_read(priv->regmap, SX1301_FORCE_CTRL, &val);
 	if (ret) {
 		dev_err(priv->dev, "0|105 read failed\n");
 		return ret;
@@ -439,7 +374,7 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 
 	val &= ~REG_0_105_FORCE_HOST_RADIO_CTRL;
 
-	ret = sx1301_page_write(priv, 0, 105, val);
+	ret = regmap_write(priv->regmap, SX1301_FORCE_CTRL, val);
 	if (ret) {
 		dev_err(priv->dev, "0|105 write failed\n");
 		return ret;
@@ -449,13 +384,13 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 	if (false)
 		val |= BIT(5); /* SX1255 */
 
-	ret = sx1301_page_write(priv, 0, REG_0_RADIO_SELECT, val);
+	ret = regmap_write(priv->regmap, SX1301_CHRS, val);
 	if (ret) {
 		dev_err(priv->dev, "radio select write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(priv, 0, REG_0_MCU, &val);
+	ret = regmap_read(priv->regmap, SX1301_MCU_CTRL, &val);
 	if (ret) {
 		dev_err(priv->dev, "MCU read (0) failed\n");
 		return ret;
@@ -463,7 +398,7 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 
 	val &= ~REG_0_MCU_RST_1;
 
-	ret = sx1301_page_write(priv, 0, REG_0_MCU, val);
+	ret = regmap_write(priv->regmap, SX1301_MCU_CTRL, val);
 	if (ret) {
 		dev_err(priv->dev, "MCU write (0) failed\n");
 		return ret;
@@ -483,13 +418,7 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 		return -ENXIO;
 	}
 
-	ret = sx1301_page_switch(priv, 3);
-	if (ret) {
-		dev_err(priv->dev, "page switch 3 failed\n");
-		return ret;
-	}
-
-	ret = sx1301_read(priv, REG_EMERGENCY_FORCE, &val);
+	ret = regmap_read(priv->regmap, SX1301_EMERGENCY_FORCE_HOST_CTRL, &val);
 	if (ret) {
 		dev_err(priv->dev, "emergency force read failed\n");
 		return ret;
@@ -497,7 +426,7 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 
 	val &= ~REG_EMERGENCY_FORCE_HOST_CTRL;
 
-	ret = sx1301_write(priv, REG_EMERGENCY_FORCE, val);
+	ret = regmap_write(priv->regmap, SX1301_EMERGENCY_FORCE_HOST_CTRL, val);
 	if (ret) {
 		dev_err(priv->dev, "emergency force write failed\n");
 		return ret;
@@ -506,7 +435,7 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 	dev_err(priv->dev, "starting calibration...\n");
 	msleep(2300);
 
-	ret = sx1301_read(priv, REG_EMERGENCY_FORCE, &val);
+	ret = regmap_read(priv->regmap, SX1301_EMERGENCY_FORCE_HOST_CTRL, &val);
 	if (ret) {
 		dev_err(priv->dev, "emergency force read (1) failed\n");
 		return ret;
@@ -514,13 +443,13 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 
 	val |= REG_EMERGENCY_FORCE_HOST_CTRL;
 
-	ret = sx1301_write(priv, REG_EMERGENCY_FORCE, val);
+	ret = regmap_write(priv->regmap, SX1301_EMERGENCY_FORCE_HOST_CTRL, val);
 	if (ret) {
 		dev_err(priv->dev, "emergency force write (1) failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(priv, REG_MCU_AGC_STATUS, &val);
+	ret = regmap_read(priv->regmap, SX1301_AGCSTS, &val);
 	if (ret) {
 		dev_err(priv->dev, "AGC status read failed\n");
 		return ret;
@@ -538,7 +467,7 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 {
 	const struct firmware *fw;
-	u8 val;
+	unsigned int val;
 	int ret;
 
 	ret = request_firmware(&fw, "sx1301_arb.bin", priv->dev);
@@ -563,7 +492,7 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 	if (ret)
 		return ret;
 
-	ret = sx1301_page_read(priv, 0, 105, &val);
+	ret = regmap_read(priv->regmap, SX1301_FORCE_CTRL, &val);
 	if (ret) {
 		dev_err(priv->dev, "0|105 read failed\n");
 		return ret;
@@ -571,19 +500,19 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 
 	val &= ~(REG_0_105_FORCE_HOST_RADIO_CTRL | REG_0_105_FORCE_HOST_FE_CTRL | REG_0_105_FORCE_DEC_FILTER_GAIN);
 
-	ret = sx1301_page_write(priv, 0, 105, val);
+	ret = regmap_write(priv->regmap, SX1301_FORCE_CTRL, val);
 	if (ret) {
 		dev_err(priv->dev, "0|105 write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_write(priv, 0, REG_0_RADIO_SELECT, 0);
+	ret = regmap_write(priv->regmap, SX1301_CHRS, 0);
 	if (ret) {
 		dev_err(priv->dev, "radio select write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(priv, 0, REG_0_MCU, &val);
+	ret = regmap_read(priv->regmap, SX1301_MCU_CTRL, &val);
 	if (ret) {
 		dev_err(priv->dev, "MCU read (0) failed\n");
 		return ret;
@@ -591,7 +520,7 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 
 	val &= ~(REG_0_MCU_RST_1 | REG_0_MCU_RST_0);
 
-	ret = sx1301_page_write(priv, 0, REG_0_MCU, val);
+	ret = regmap_write(priv->regmap, SX1301_MCU_CTRL, val);
 	if (ret) {
 		dev_err(priv->dev, "MCU write (0) failed\n");
 		return ret;
@@ -645,7 +574,7 @@ static int sx1301_probe(struct spi_device *spi)
 	struct gpio_desc *rst;
 	int ret;
 	unsigned int ver;
-	u8 val;
+	unsigned int val;
 
 	rst = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
 	if (IS_ERR(rst))
@@ -665,7 +594,6 @@ static int sx1301_probe(struct spi_device *spi)
 
 	priv = netdev_priv(netdev);
 	priv->rst_gpio = rst;
-	priv->cur_page = 0xff;
 
 	spi_set_drvdata(spi, priv);
 	priv->dev = &spi->dev;
@@ -690,7 +618,7 @@ static int sx1301_probe(struct spi_device *spi)
 		return -ENXIO;
 	}
 
-	ret = sx1301_write(priv, REG_PAGE_RESET, 0);
+	ret = regmap_write(priv->regmap, SX1301_PAGE, 0);
 	if (ret) {
 		dev_err(&spi->dev, "page/reset write failed\n");
 		return ret;
@@ -702,7 +630,7 @@ static int sx1301_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	ret = sx1301_read(priv, 16, &val);
+	ret = regmap_read(priv->regmap, SX1301_GEN, &val);
 	if (ret) {
 		dev_err(&spi->dev, "16 read failed\n");
 		return ret;
@@ -710,13 +638,13 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val &= ~REG_16_GLOBAL_EN;
 
-	ret = sx1301_write(priv, 16, val);
+	ret = regmap_write(priv->regmap, SX1301_GEN, val);
 	if (ret) {
 		dev_err(&spi->dev, "16 write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(priv, 17, &val);
+	ret = regmap_read(priv->regmap, SX1301_CKEN, &val);
 	if (ret) {
 		dev_err(&spi->dev, "17 read failed\n");
 		return ret;
@@ -724,13 +652,13 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val &= ~REG_17_CLK32M_EN;
 
-	ret = sx1301_write(priv, 17, val);
+	ret = regmap_write(priv->regmap, SX1301_CKEN, val);
 	if (ret) {
 		dev_err(&spi->dev, "17 write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(priv, 2, 43, &val);
+	ret = regmap_read(priv->regmap, SX1301_RADIO_CFG, &val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 read failed\n");
 		return ret;
@@ -738,7 +666,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val |= REG_2_43_RADIO_B_EN | REG_2_43_RADIO_A_EN;
 
-	ret = sx1301_page_write(priv, 2, 43, val);
+	ret = regmap_write(priv->regmap, SX1301_RADIO_CFG, val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 write failed\n");
 		return ret;
@@ -746,7 +674,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	msleep(500);
 
-	ret = sx1301_page_read(priv, 2, 43, &val);
+	ret = regmap_read(priv->regmap, SX1301_RADIO_CFG, &val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 read failed\n");
 		return ret;
@@ -754,7 +682,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val |= REG_2_43_RADIO_RST;
 
-	ret = sx1301_page_write(priv, 2, 43, val);
+	ret = regmap_write(priv->regmap, SX1301_RADIO_CFG, val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 write failed\n");
 		return ret;
@@ -762,7 +690,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	msleep(5);
 
-	ret = sx1301_page_read(priv, 2, 43, &val);
+	ret = regmap_read(priv->regmap, SX1301_RADIO_CFG, &val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 read failed\n");
 		return ret;
@@ -770,7 +698,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val &= ~REG_2_43_RADIO_RST;
 
-	ret = sx1301_page_write(priv, 2, 43, val);
+	ret = regmap_write(priv->regmap, SX1301_RADIO_CFG, val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 write failed\n");
 		return ret;
@@ -787,8 +715,7 @@ static int sx1301_probe(struct spi_device *spi)
 	priv->radio_a_ctrl->dev.of_node = of_get_child_by_name(spi->dev.of_node, "radio-a");
 
 	radio = spi_controller_get_devdata(priv->radio_a_ctrl);
-	radio->page = 2;
-	radio->regs = REG_2_SPI_RADIO_A_DATA;
+	radio->regs = SX1301_RADIO_A_SPI_DATA;
 	radio->parent = spi;
 
 	ret = devm_spi_register_controller(&spi->dev, priv->radio_a_ctrl);
@@ -809,8 +736,7 @@ static int sx1301_probe(struct spi_device *spi)
 	priv->radio_b_ctrl->dev.of_node = of_get_child_by_name(spi->dev.of_node, "radio-b");
 
 	radio = spi_controller_get_devdata(priv->radio_b_ctrl);
-	radio->page = 2;
-	radio->regs = REG_2_SPI_RADIO_B_DATA;
+	radio->regs = SX1301_RADIO_B_SPI_DATA;
 	radio->parent = spi;
 
 	ret = devm_spi_register_controller(&spi->dev, priv->radio_b_ctrl);
@@ -822,7 +748,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	/* GPIO */
 
-	ret = sx1301_read(priv, REG_GPIO_MODE, &val);
+	ret = regmap_read(priv->regmap, SX1301_GPMODE, &val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO mode read failed\n");
 		return ret;
@@ -830,13 +756,13 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val |= GENMASK(4, 0);
 
-	ret = sx1301_write(priv, REG_GPIO_MODE, val);
+	ret = regmap_write(priv->regmap, SX1301_GPMODE, val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO mode write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(priv, REG_GPIO_SELECT_OUTPUT, &val);
+	ret = regmap_read(priv->regmap, SX1301_GPSO, &val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO select output read failed\n");
 		return ret;
@@ -845,7 +771,7 @@ static int sx1301_probe(struct spi_device *spi)
 	val &= ~GENMASK(3, 0);
 	val |= 2;
 
-	ret = sx1301_write(priv, REG_GPIO_SELECT_OUTPUT, val);
+	ret = regmap_write(priv->regmap, SX1301_GPSO, val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO select output write failed\n");
 		return ret;
@@ -853,7 +779,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	/* TODO LBT */
 
-	ret = sx1301_read(priv, 16, &val);
+	ret = regmap_read(priv->regmap, SX1301_GEN, &val);
 	if (ret) {
 		dev_err(&spi->dev, "16 read (1) failed\n");
 		return ret;
@@ -861,13 +787,13 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val |= REG_16_GLOBAL_EN;
 
-	ret = sx1301_write(priv, 16, val);
+	ret = regmap_write(priv->regmap, SX1301_GEN, val);
 	if (ret) {
 		dev_err(&spi->dev, "16 write (1) failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(priv, 17, &val);
+	ret = regmap_read(priv->regmap, SX1301_CKEN, &val);
 	if (ret) {
 		dev_err(&spi->dev, "17 read (1) failed\n");
 		return ret;
@@ -875,7 +801,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val |= REG_17_CLK32M_EN;
 
-	ret = sx1301_write(priv, 17, val);
+	ret = regmap_write(priv->regmap, SX1301_CKEN, val);
 	if (ret) {
 		dev_err(&spi->dev, "17 write (1) failed\n");
 		return ret;
diff --git a/drivers/net/lora/sx1301.h b/drivers/net/lora/sx1301.h
index 2fc283f..b21e5c6 100644
--- a/drivers/net/lora/sx1301.h
+++ b/drivers/net/lora/sx1301.h
@@ -18,11 +18,41 @@
 /* Page independent */
 #define SX1301_PAGE     0x00
 #define SX1301_VER      0x01
+#define SX1301_MPA      0x09
+#define SX1301_MPD      0x0A
+#define SX1301_GEN      0x10
+#define SX1301_CKEN     0x11
+#define SX1301_GPSO     0x1C
+#define SX1301_GPMODE   0x1D
+#define SX1301_AGCSTS   0x20
 
 #define SX1301_VIRT_BASE    0x100
 #define SX1301_PAGE_LEN     0x80
 #define SX1301_PAGE_BASE(n) (SX1301_VIRT_BASE + (SX1301_PAGE_LEN * n))
 
+/* Page 0 */
+#define SX1301_CHRS         (SX1301_PAGE_BASE(0) + 0x23)
+#define SX1301_FORCE_CTRL   (SX1301_PAGE_BASE(0) + 0x69)
+#define SX1301_MCU_CTRL     (SX1301_PAGE_BASE(0) + 0x6A)
+
+/* Page 2 */
+#define SX1301_RADIO_A_SPI_DATA     (SX1301_PAGE_BASE(2) + 0x21)
+#define SX1301_RADIO_A_SPI_DATA_RB  (SX1301_PAGE_BASE(2) + 0x22)
+#define SX1301_RADIO_A_SPI_ADDR     (SX1301_PAGE_BASE(2) + 0x23)
+#define SX1301_RADIO_A_SPI_CS       (SX1301_PAGE_BASE(2) + 0x25)
+#define SX1301_RADIO_B_SPI_DATA     (SX1301_PAGE_BASE(2) + 0x26)
+#define SX1301_RADIO_B_SPI_DATA_RB  (SX1301_PAGE_BASE(2) + 0x27)
+#define SX1301_RADIO_B_SPI_ADDR     (SX1301_PAGE_BASE(2) + 0x28)
+#define SX1301_RADIO_B_SPI_CS       (SX1301_PAGE_BASE(2) + 0x2A)
+#define SX1301_RADIO_CFG            (SX1301_PAGE_BASE(2) + 0x2B)
+#define SX1301_DBG_ARB_MCU_RAM_DATA (SX1301_PAGE_BASE(2) + 0x40)
+#define SX1301_DBG_AGC_MCU_RAM_DATA (SX1301_PAGE_BASE(2) + 0x41)
+#define SX1301_DBG_ARB_MCU_RAM_ADDR (SX1301_PAGE_BASE(2) + 0x50)
+#define SX1301_DBG_AGC_MCU_RAM_ADDR (SX1301_PAGE_BASE(2) + 0x51)
+
+/* Page 3 */
+#define SX1301_EMERGENCY_FORCE_HOST_CTRL (SX1301_PAGE_BASE(3) + 0x7F)
+
 #define SX1301_MAX_REGISTER         (SX1301_PAGE_BASE(3) + 0x7F)
 
 #endif
-- 
2.7.4

^ permalink raw reply related

* [PATCH lora-next v2 7/8] net: lora: sx1301: add initial registration for regmap
From: Ben Whitten @ 2018-08-09 12:33 UTC (permalink / raw)
  To: afaerber; +Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten
In-Reply-To: <1533818018-29005-1-git-send-email-ben.whitten@lairdtech.com>

The register and bit-field definitions are taken from the SX1301
datasheet version 2.01 dated June 2014 with the revision information
'First released version'.

The reset state and RW capability of each field is not reflected in this
patch however from the datasheet:
"Bits and registers that are not documented are reserved. They may
include calibration values. It is important not to modify these bits and
registers. If specific bits must be changed in a register with reserved
bits, the register must be read first, specific bits modified while
masking reserved bits and then the register can be written."

Then goes on to state:
"Reserved bits should be written with their reset state, they may be
read different states."

Caching is currently disabled.

The version is read back using regmap_read to verify regmap operation,
in doing so needs to be moved after priv and regmap allocation.

Further registers or fields are added as they are required in conversion.

Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
---
 drivers/net/lora/Kconfig  |  1 +
 drivers/net/lora/sx1301.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
 drivers/net/lora/sx1301.h | 10 ++++++++++
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/net/lora/Kconfig b/drivers/net/lora/Kconfig
index bb57a01..79d23f2 100644
--- a/drivers/net/lora/Kconfig
+++ b/drivers/net/lora/Kconfig
@@ -49,6 +49,7 @@ config LORA_SX1301
 	tristate "Semtech SX1301 SPI driver"
 	default y
 	depends on SPI
+	select REGMAP_SPI
 	help
 	  Semtech SX1301
 
diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 8e81179..766df06 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -20,11 +20,11 @@
 #include <linux/of_gpio.h>
 #include <linux/lora/dev.h>
 #include <linux/spi/spi.h>
+#include <linux/regmap.h>
 
 #include "sx1301.h"
 
 #define REG_PAGE_RESET			0
-#define REG_VERSION			1
 #define REG_MCU_PROM_ADDR		9
 #define REG_MCU_PROM_DATA		10
 #define REG_GPIO_SELECT_INPUT		27
@@ -68,6 +68,35 @@
 
 #define REG_EMERGENCY_FORCE_HOST_CTRL	BIT(0)
 
+static const struct regmap_range_cfg sx1301_ranges[] = {
+	{
+		.name = "Pages",
+
+		.range_min = SX1301_VIRT_BASE,
+		.range_max = SX1301_MAX_REGISTER,
+
+		.selector_reg = SX1301_PAGE,
+		.selector_mask = 0x3,
+
+		.window_start = 0,
+		.window_len = SX1301_PAGE_LEN,
+	},
+};
+
+static struct regmap_config sx1301_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.cache_type = REGCACHE_NONE,
+
+	.read_flag_mask = 0,
+	.write_flag_mask = BIT(7),
+
+	.ranges = sx1301_ranges,
+	.num_ranges = ARRAY_SIZE(sx1301_ranges),
+	.max_register = SX1301_MAX_REGISTER,
+};
+
 struct spi_sx1301 {
 	struct spi_device *parent;
 	u8 page;
@@ -81,6 +110,7 @@ struct sx1301_priv {
 	struct gpio_desc *rst_gpio;
 	u8 cur_page;
 	struct spi_controller *radio_a_ctrl, *radio_b_ctrl;
+	struct regmap		*regmap;
 };
 
 static int sx1301_read_burst(struct sx1301_priv *priv, u8 reg, u8 *val, size_t len)
@@ -614,6 +644,7 @@ static int sx1301_probe(struct spi_device *spi)
 	struct spi_sx1301 *radio;
 	struct gpio_desc *rst;
 	int ret;
+	unsigned int ver;
 	u8 val;
 
 	rst = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
@@ -641,14 +672,21 @@ static int sx1301_probe(struct spi_device *spi)
 	priv->spi = spi;
 	SET_NETDEV_DEV(netdev, &spi->dev);
 
-	ret = sx1301_read(priv, REG_VERSION, &val);
+	priv->regmap = devm_regmap_init_spi(spi, &sx1301_regmap_config);
+	if (IS_ERR(priv->regmap)) {
+		ret = PTR_ERR(priv->regmap);
+		dev_err(&spi->dev, "Regmap allocation failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_read(priv->regmap, SX1301_VER, &ver);
 	if (ret) {
 		dev_err(&spi->dev, "version read failed\n");
 		return ret;
 	}
 
-	if (val != SX1301_CHIP_VERSION) {
-		dev_err(&spi->dev, "unexpected version: %u\n", val);
+	if (ver != SX1301_CHIP_VERSION) {
+		dev_err(&spi->dev, "unexpected version: %u\n", ver);
 		return -ENXIO;
 	}
 
diff --git a/drivers/net/lora/sx1301.h b/drivers/net/lora/sx1301.h
index b37ac56..2fc283f 100644
--- a/drivers/net/lora/sx1301.h
+++ b/drivers/net/lora/sx1301.h
@@ -15,4 +15,14 @@
 #define SX1301_MCU_AGC_FW_VERSION 4
 #define SX1301_MCU_AGC_CAL_FW_VERSION 2
 
+/* Page independent */
+#define SX1301_PAGE     0x00
+#define SX1301_VER      0x01
+
+#define SX1301_VIRT_BASE    0x100
+#define SX1301_PAGE_LEN     0x80
+#define SX1301_PAGE_BASE(n) (SX1301_VIRT_BASE + (SX1301_PAGE_LEN * n))
+
+#define SX1301_MAX_REGISTER         (SX1301_PAGE_BASE(3) + 0x7F)
+
 #endif
-- 
2.7.4

^ permalink raw reply related

* [PATCH lora-next v2 6/8] net: lora: sx1301: replace version and size magic numbers with defines
From: Ben Whitten @ 2018-08-09 12:33 UTC (permalink / raw)
  To: afaerber; +Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten
In-Reply-To: <1533818018-29005-1-git-send-email-ben.whitten@lairdtech.com>

We replace the hard coded numbers for size and version with meaningful
names.

Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
---
 drivers/net/lora/sx1301.c | 21 +++++++++++++--------
 drivers/net/lora/sx1301.h | 18 ++++++++++++++++++
 2 files changed, 31 insertions(+), 8 deletions(-)
 create mode 100644 drivers/net/lora/sx1301.h

diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 916ee40..8e81179 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -21,6 +21,8 @@
 #include <linux/lora/dev.h>
 #include <linux/spi/spi.h>
 
+#include "sx1301.h"
+
 #define REG_PAGE_RESET			0
 #define REG_VERSION			1
 #define REG_MCU_PROM_ADDR		9
@@ -293,7 +295,7 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 	u8 val, rst, select_mux;
 	int ret;
 
-	if (fw->size != 8192) {
+	if (fw->size != SX1301_MCU_FW_BYTE) {
 		dev_err(priv->dev, "Unexpected firmware size\n");
 		return -EINVAL;
 	}
@@ -445,8 +447,9 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 
 	dev_info(priv->dev, "AGC calibration firmware version %u\n", (unsigned)val);
 
-	if (val != 2) {
-		dev_err(priv->dev, "unexpected firmware version, expecting %u\n", 2);
+	if (val != SX1301_MCU_AGC_CAL_FW_VERSION) {
+		dev_err(priv->dev, "unexpected firmware version, expecting %u\n",
+				SX1301_MCU_AGC_CAL_FW_VERSION);
 		return -ENXIO;
 	}
 
@@ -572,8 +575,9 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 
 	dev_info(priv->dev, "AGC firmware version %u\n", (unsigned)val);
 
-	if (val != 4) {
-		dev_err(priv->dev, "unexpected firmware version, expecting %u\n", 4);
+	if (val != SX1301_MCU_AGC_FW_VERSION) {
+		dev_err(priv->dev, "unexpected firmware version, expecting %u\n",
+				SX1301_MCU_AGC_FW_VERSION);
 		return -ENXIO;
 	}
 
@@ -585,8 +589,9 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 
 	dev_info(priv->dev, "ARB firmware version %u\n", (unsigned)val);
 
-	if (val != 1) {
-		dev_err(priv->dev, "unexpected firmware version, expecting %u\n", 1);
+	if (val != SX1301_MCU_ARB_FW_VERSION) {
+		dev_err(priv->dev, "unexpected firmware version, expecting %u\n",
+				SX1301_MCU_ARB_FW_VERSION);
 		return -ENXIO;
 	}
 
@@ -642,7 +647,7 @@ static int sx1301_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	if (val != 103) {
+	if (val != SX1301_CHIP_VERSION) {
 		dev_err(&spi->dev, "unexpected version: %u\n", val);
 		return -ENXIO;
 	}
diff --git a/drivers/net/lora/sx1301.h b/drivers/net/lora/sx1301.h
new file mode 100644
index 0000000..b37ac56
--- /dev/null
+++ b/drivers/net/lora/sx1301.h
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Semtech SX1301 lora concentrator
+ *
+ * Copyright (c) 2018   Ben Whitten
+ */
+
+#ifndef _SX1301_
+#define _SX1301_
+
+#define SX1301_CHIP_VERSION 103
+
+#define SX1301_MCU_FW_BYTE 8192
+#define SX1301_MCU_ARB_FW_VERSION 1
+#define SX1301_MCU_AGC_FW_VERSION 4
+#define SX1301_MCU_AGC_CAL_FW_VERSION 2
+
+#endif
-- 
2.7.4

^ permalink raw reply related

* [PATCH lora-next v2 5/8] net: lora: sx1301: remove duplicate firmware size checks
From: Ben Whitten @ 2018-08-09 12:33 UTC (permalink / raw)
  To: afaerber; +Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten
In-Reply-To: <1533818018-29005-1-git-send-email-ben.whitten@lairdtech.com>

No need to check the size of the firmware multiple times, just do it once
in the function responsible for loading as the firmwares are the same size.

Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
---
 drivers/net/lora/sx1301.c | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 3f2a532..916ee40 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -293,8 +293,10 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct
 	u8 val, rst, select_mux;
 	int ret;
 
-	if (fw->size > 8192)
+	if (fw->size != 8192) {
+		dev_err(priv->dev, "Unexpected firmware size\n");
 		return -EINVAL;
+	}
 
 	switch (mcu) {
 	case 0:
@@ -390,11 +392,6 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 		return ret;
 	}
 
-	if (fw->size != 8192) {
-		dev_err(priv->dev, "unexpected agc cal firmware size\n");
-		return -EINVAL;
-	}
-
 	ret = sx1301_load_firmware(priv, 1, fw);
 	release_firmware(fw);
 	if (ret) {
@@ -517,12 +514,6 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 		return ret;
 	}
 
-	if (fw->size != 8192) {
-		dev_err(priv->dev, "unexpected arb firmware size\n");
-		release_firmware(fw);
-		return -EINVAL;
-	}
-
 	ret = sx1301_load_firmware(priv, 0, fw);
 	release_firmware(fw);
 	if (ret)
@@ -534,12 +525,6 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 		return ret;
 	}
 
-	if (fw->size != 8192) {
-		dev_err(priv->dev, "unexpected agc firmware size\n");
-		release_firmware(fw);
-		return -EINVAL;
-	}
-
 	ret = sx1301_load_firmware(priv, 1, fw);
 	release_firmware(fw);
 	if (ret)
-- 
2.7.4

^ permalink raw reply related

* [PATCH lora-next v2 3/8] net: lora: sx1301: convert to passing priv data throughout
From: Ben Whitten @ 2018-08-09 12:33 UTC (permalink / raw)
  To: afaerber; +Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten
In-Reply-To: <1533818018-29005-1-git-send-email-ben.whitten@lairdtech.com>

Instead of passing around the spi device we instead pass around our
driver data directly.

Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
---
 drivers/net/lora/sx1301.c | 305 +++++++++++++++++++++++-----------------------
 1 file changed, 155 insertions(+), 150 deletions(-)

diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 3c09f5a..7324001 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -73,24 +73,26 @@ struct spi_sx1301 {
 };
 
 struct sx1301_priv {
+	struct device		*dev;
+	struct spi_device	*spi;
 	struct lora_priv lora;
 	struct gpio_desc *rst_gpio;
 	u8 cur_page;
 	struct spi_controller *radio_a_ctrl, *radio_b_ctrl;
 };
 
-static int sx1301_read_burst(struct spi_device *spi, u8 reg, u8 *val, size_t len)
+static int sx1301_read_burst(struct sx1301_priv *priv, u8 reg, u8 *val, size_t len)
 {
 	u8 addr = reg & 0x7f;
-	return spi_write_then_read(spi, &addr, 1, val, len);
+	return spi_write_then_read(priv->spi, &addr, 1, val, len);
 }
 
-static int sx1301_read(struct spi_device *spi, u8 reg, u8 *val)
+static int sx1301_read(struct sx1301_priv *priv, u8 reg, u8 *val)
 {
-	return sx1301_read_burst(spi, reg, val, 1);
+	return sx1301_read_burst(priv, reg, val, 1);
 }
 
-static int sx1301_write_burst(struct spi_device *spi, u8 reg, const u8 *val, size_t len)
+static int sx1301_write_burst(struct sx1301_priv *priv, u8 reg, const u8 *val, size_t len)
 {
 	u8 addr = reg | BIT(7);
 	struct spi_transfer xfr[2] = {
@@ -98,26 +100,25 @@ static int sx1301_write_burst(struct spi_device *spi, u8 reg, const u8 *val, siz
 		{ .tx_buf = val, .len = len },
 	};
 
-	return spi_sync_transfer(spi, xfr, 2);
+	return spi_sync_transfer(priv->spi, xfr, 2);
 }
 
-static int sx1301_write(struct spi_device *spi, u8 reg, u8 val)
+static int sx1301_write(struct sx1301_priv *priv, u8 reg, u8 val)
 {
-	return sx1301_write_burst(spi, reg, &val, 1);
+	return sx1301_write_burst(priv, reg, &val, 1);
 }
 
-static int sx1301_page_switch(struct spi_device *spi, u8 page)
+static int sx1301_page_switch(struct sx1301_priv *priv, u8 page)
 {
-	struct sx1301_priv *priv = spi_get_drvdata(spi);
 	int ret;
 
 	if (priv->cur_page == page)
 		return 0;
 
-	dev_dbg(&spi->dev, "switching to page %u\n", (unsigned)page);
-	ret = sx1301_write(spi, REG_PAGE_RESET, page & 0x3);
+	dev_dbg(priv->dev, "switching to page %u\n", (unsigned)page);
+	ret = sx1301_write(priv, REG_PAGE_RESET, page & 0x3);
 	if (ret) {
-		dev_err(&spi->dev, "switching to page %u failed\n", (unsigned)page);
+		dev_err(priv->dev, "switching to page %u failed\n", (unsigned)page);
 		return ret;
 	}
 
@@ -126,31 +127,31 @@ static int sx1301_page_switch(struct spi_device *spi, u8 page)
 	return 0;
 }
 
-static int sx1301_page_read(struct spi_device *spi, u8 page, u8 reg, u8 *val)
+static int sx1301_page_read(struct sx1301_priv *priv, u8 page, u8 reg, u8 *val)
 {
 	int ret;
 
-	ret = sx1301_page_switch(spi, page);
+	ret = sx1301_page_switch(priv, page);
 	if (ret)
 		return ret;
 
-	return sx1301_read(spi, reg, val);
+	return sx1301_read(priv, reg, val);
 }
 
-static int sx1301_page_write(struct spi_device *spi, u8 page, u8 reg, u8 val)
+static int sx1301_page_write(struct sx1301_priv *priv, u8 page, u8 reg, u8 val)
 {
 	int ret;
 
-	ret = sx1301_page_switch(spi, page);
+	ret = sx1301_page_switch(priv, page);
 	if (ret)
 		return ret;
 
-	return sx1301_write(spi, reg, val);
+	return sx1301_write(priv, reg, val);
 }
 
-static int sx1301_soft_reset(struct spi_device *spi)
+static int sx1301_soft_reset(struct sx1301_priv *priv)
 {
-	return sx1301_write(spi, REG_PAGE_RESET, REG_PAGE_RESET_SOFT_RESET);
+	return sx1301_write(priv, REG_PAGE_RESET, REG_PAGE_RESET_SOFT_RESET);
 }
 
 #define REG_RADIO_X_DATA		0
@@ -161,12 +162,13 @@ static int sx1301_soft_reset(struct spi_device *spi)
 static int sx1301_radio_set_cs(struct spi_controller *ctrl, bool enable)
 {
 	struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
+	struct sx1301_priv *priv = spi_get_drvdata(ssx->parent);
 	u8 cs;
 	int ret;
 
 	dev_dbg(&ctrl->dev, "setting CS to %s\n", enable ? "1" : "0");
 
-	ret = sx1301_page_read(ssx->parent, ssx->page, ssx->regs + REG_RADIO_X_CS, &cs);
+	ret = sx1301_page_read(priv, ssx->page, ssx->regs + REG_RADIO_X_CS, &cs);
 	if (ret) {
 		dev_warn(&ctrl->dev, "failed to read CS (%d)\n", ret);
 		cs = 0;
@@ -177,7 +179,7 @@ static int sx1301_radio_set_cs(struct spi_controller *ctrl, bool enable)
 	else
 		cs &= ~BIT(0);
 
-	ret = sx1301_page_write(ssx->parent, ssx->page, ssx->regs + REG_RADIO_X_CS, cs);
+	ret = sx1301_page_write(priv, ssx->page, ssx->regs + REG_RADIO_X_CS, cs);
 	if (ret) {
 		dev_err(&ctrl->dev, "failed to write CS (%d)\n", ret);
 		return ret;
@@ -200,6 +202,7 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
 	struct spi_device *spi, struct spi_transfer *xfr)
 {
 	struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
+	struct sx1301_priv *priv = spi_get_drvdata(ssx->parent);
 	const u8 *tx_buf = xfr->tx_buf;
 	u8 *rx_buf = xfr->rx_buf;
 	int ret;
@@ -210,13 +213,13 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
 	dev_dbg(&spi->dev, "transferring one (%u)\n", xfr->len);
 
 	if (tx_buf) {
-		ret = sx1301_page_write(ssx->parent, ssx->page, ssx->regs + REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
+		ret = sx1301_page_write(priv, ssx->page, ssx->regs + REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
 		if (ret) {
 			dev_err(&spi->dev, "SPI radio address write failed\n");
 			return ret;
 		}
 
-		ret = sx1301_page_write(ssx->parent, ssx->page, ssx->regs + REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
+		ret = sx1301_page_write(priv, ssx->page, ssx->regs + REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
 		if (ret) {
 			dev_err(&spi->dev, "SPI radio data write failed\n");
 			return ret;
@@ -236,7 +239,7 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
 	}
 
 	if (rx_buf) {
-		ret = sx1301_page_read(ssx->parent, ssx->page, ssx->regs + REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
+		ret = sx1301_page_read(priv, ssx->page, ssx->regs + REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
 		if (ret) {
 			dev_err(&spi->dev, "SPI radio data read failed\n");
 			return ret;
@@ -246,45 +249,45 @@ static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
 	return 0;
 }
 
-static int sx1301_agc_ram_read(struct spi_device *spi, u8 addr, u8 *val)
+static int sx1301_agc_ram_read(struct sx1301_priv *priv, u8 addr, u8 *val)
 {
 	int ret;
 
-	ret = sx1301_page_write(spi, 2, REG_2_DBG_AGC_MCU_RAM_ADDR, addr);
+	ret = sx1301_page_write(priv, 2, REG_2_DBG_AGC_MCU_RAM_ADDR, addr);
 	if (ret) {
-		dev_err(&spi->dev, "AGC RAM addr write failed\n");
+		dev_err(priv->dev, "AGC RAM addr write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(spi, 2, REG_2_DBG_AGC_MCU_RAM_DATA, val);
+	ret = sx1301_page_read(priv, 2, REG_2_DBG_AGC_MCU_RAM_DATA, val);
 	if (ret) {
-		dev_err(&spi->dev, "AGC RAM data read failed\n");
+		dev_err(priv->dev, "AGC RAM data read failed\n");
 		return ret;
 	}
 
 	return 0;
 }
 
-static int sx1301_arb_ram_read(struct spi_device *spi, u8 addr, u8 *val)
+static int sx1301_arb_ram_read(struct sx1301_priv *priv, u8 addr, u8 *val)
 {
 	int ret;
 
-	ret = sx1301_page_write(spi, 2, REG_2_DBG_ARB_MCU_RAM_ADDR, addr);
+	ret = sx1301_page_write(priv, 2, REG_2_DBG_ARB_MCU_RAM_ADDR, addr);
 	if (ret) {
-		dev_err(&spi->dev, "ARB RAM addr write failed\n");
+		dev_err(priv->dev, "ARB RAM addr write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(spi, 2, REG_2_DBG_ARB_MCU_RAM_DATA, val);
+	ret = sx1301_page_read(priv, 2, REG_2_DBG_ARB_MCU_RAM_DATA, val);
 	if (ret) {
-		dev_err(&spi->dev, "ARB RAM data read failed\n");
+		dev_err(priv->dev, "ARB RAM data read failed\n");
 		return ret;
 	}
 
 	return 0;
 }
 
-static int sx1301_load_firmware(struct spi_device *spi, int mcu, const u8 *data, size_t len)
+static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const u8 *data, size_t len)
 {
 	u8 *buf;
 	u8 val, rst, select_mux;
@@ -306,36 +309,36 @@ static int sx1301_load_firmware(struct spi_device *spi, int mcu, const u8 *data,
 		return -EINVAL;
 	}
 
-	ret = sx1301_page_read(spi, 0, REG_0_MCU, &val);
+	ret = sx1301_page_read(priv, 0, REG_0_MCU, &val);
 	if (ret) {
-		dev_err(&spi->dev, "MCU read failed\n");
+		dev_err(priv->dev, "MCU read failed\n");
 		return ret;
 	}
 
 	val |= rst;
 	val &= ~select_mux;
 
-	ret = sx1301_page_write(spi, 0, REG_0_MCU, val);
+	ret = sx1301_page_write(priv, 0, REG_0_MCU, val);
 	if (ret) {
-		dev_err(&spi->dev, "MCU reset / select mux write failed\n");
+		dev_err(priv->dev, "MCU reset / select mux write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_write(spi, REG_MCU_PROM_ADDR, 0);
+	ret = sx1301_write(priv, REG_MCU_PROM_ADDR, 0);
 	if (ret) {
-		dev_err(&spi->dev, "MCU prom addr write failed\n");
+		dev_err(priv->dev, "MCU prom addr write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_write_burst(spi, REG_MCU_PROM_DATA, data, len);
+	ret = sx1301_write_burst(priv, REG_MCU_PROM_DATA, data, len);
 	if (ret) {
-		dev_err(&spi->dev, "MCU prom data write failed\n");
+		dev_err(priv->dev, "MCU prom data write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(spi, REG_MCU_PROM_DATA, &val);
+	ret = sx1301_read(priv, REG_MCU_PROM_DATA, &val);
 	if (ret) {
-		dev_err(&spi->dev, "MCU prom data dummy read failed\n");
+		dev_err(priv->dev, "MCU prom data dummy read failed\n");
 		return ret;
 	}
 
@@ -343,73 +346,73 @@ static int sx1301_load_firmware(struct spi_device *spi, int mcu, const u8 *data,
 	if (!buf)
 		return -ENOMEM;
 
-	ret = sx1301_read_burst(spi, REG_MCU_PROM_DATA, buf, len);
+	ret = sx1301_read_burst(priv, REG_MCU_PROM_DATA, buf, len);
 	if (ret) {
-		dev_err(&spi->dev, "MCU prom data read failed\n");
+		dev_err(priv->dev, "MCU prom data read failed\n");
 		kfree(buf);
 		return ret;
 	}
 
 	if (memcmp(data, buf, len)) {
-		dev_err(&spi->dev, "MCU prom data read does not match data written\n");
+		dev_err(priv->dev, "MCU prom data read does not match data written\n");
 		kfree(buf);
 		return -ENXIO;
 	}
 
 	kfree(buf);
 
-	ret = sx1301_page_read(spi, 0, REG_0_MCU, &val);
+	ret = sx1301_page_read(priv, 0, REG_0_MCU, &val);
 	if (ret) {
-		dev_err(&spi->dev, "MCU read (1) failed\n");
+		dev_err(priv->dev, "MCU read (1) failed\n");
 		return ret;
 	}
 
 	val |= select_mux;
 
-	ret = sx1301_page_write(spi, 0, REG_0_MCU, val);
+	ret = sx1301_page_write(priv, 0, REG_0_MCU, val);
 	if (ret) {
-		dev_err(&spi->dev, "MCU reset / select mux write (1) failed\n");
+		dev_err(priv->dev, "MCU reset / select mux write (1) failed\n");
 		return ret;
 	}
 
 	return 0;
 }
 
-static int sx1301_agc_calibrate(struct spi_device *spi)
+static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 {
 	const struct firmware *fw;
 	u8 val;
 	int ret;
 
-	ret = request_firmware(&fw, "sx1301_agc_calibration.bin", &spi->dev);
+	ret = request_firmware(&fw, "sx1301_agc_calibration.bin", priv->dev);
 	if (ret) {
-		dev_err(&spi->dev, "agc cal firmware file load failed\n");
+		dev_err(priv->dev, "agc cal firmware file load failed\n");
 		return ret;
 	}
 
 	if (fw->size != 8192) {
-		dev_err(&spi->dev, "unexpected agc cal firmware size\n");
+		dev_err(priv->dev, "unexpected agc cal firmware size\n");
 		return -EINVAL;
 	}
 
-	ret = sx1301_load_firmware(spi, 1, fw->data, fw->size);
+	ret = sx1301_load_firmware(priv, 1, fw->data, fw->size);
 	release_firmware(fw);
 	if (ret) {
-		dev_err(&spi->dev, "agc cal firmware load failed\n");
+		dev_err(priv->dev, "agc cal firmware load failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(spi, 0, 105, &val);
+	ret = sx1301_page_read(priv, 0, 105, &val);
 	if (ret) {
-		dev_err(&spi->dev, "0|105 read failed\n");
+		dev_err(priv->dev, "0|105 read failed\n");
 		return ret;
 	}
 
 	val &= ~REG_0_105_FORCE_HOST_RADIO_CTRL;
 
-	ret = sx1301_page_write(spi, 0, 105, val);
+	ret = sx1301_page_write(priv, 0, 105, val);
 	if (ret) {
-		dev_err(&spi->dev, "0|105 write failed\n");
+		dev_err(priv->dev, "0|105 write failed\n");
 		return ret;
 	}
 
@@ -417,188 +420,188 @@ static int sx1301_agc_calibrate(struct spi_device *spi)
 	if (false)
 		val |= BIT(5); /* SX1255 */
 
-	ret = sx1301_page_write(spi, 0, REG_0_RADIO_SELECT, val);
+	ret = sx1301_page_write(priv, 0, REG_0_RADIO_SELECT, val);
 	if (ret) {
-		dev_err(&spi->dev, "radio select write failed\n");
+		dev_err(priv->dev, "radio select write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(spi, 0, REG_0_MCU, &val);
+	ret = sx1301_page_read(priv, 0, REG_0_MCU, &val);
 	if (ret) {
-		dev_err(&spi->dev, "MCU read (0) failed\n");
+		dev_err(priv->dev, "MCU read (0) failed\n");
 		return ret;
 	}
 
 	val &= ~REG_0_MCU_RST_1;
 
-	ret = sx1301_page_write(spi, 0, REG_0_MCU, val);
+	ret = sx1301_page_write(priv, 0, REG_0_MCU, val);
 	if (ret) {
-		dev_err(&spi->dev, "MCU write (0) failed\n");
+		dev_err(priv->dev, "MCU write (0) failed\n");
 		return ret;
 	}
 
-	ret = sx1301_agc_ram_read(spi, 0x20, &val);
+	ret = sx1301_agc_ram_read(priv, 0x20, &val);
 	if (ret) {
-		dev_err(&spi->dev, "AGC RAM data read failed\n");
+		dev_err(priv->dev, "AGC RAM data read failed\n");
 		return ret;
 	}
 
-	dev_info(&spi->dev, "AGC calibration firmware version %u\n", (unsigned)val);
+	dev_info(priv->dev, "AGC calibration firmware version %u\n", (unsigned)val);
 
 	if (val != 2) {
-		dev_err(&spi->dev, "unexpected firmware version, expecting %u\n", 2);
+		dev_err(priv->dev, "unexpected firmware version, expecting %u\n", 2);
 		return -ENXIO;
 	}
 
-	ret = sx1301_page_switch(spi, 3);
+	ret = sx1301_page_switch(priv, 3);
 	if (ret) {
-		dev_err(&spi->dev, "page switch 3 failed\n");
+		dev_err(priv->dev, "page switch 3 failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(spi, REG_EMERGENCY_FORCE, &val);
+	ret = sx1301_read(priv, REG_EMERGENCY_FORCE, &val);
 	if (ret) {
-		dev_err(&spi->dev, "emergency force read failed\n");
+		dev_err(priv->dev, "emergency force read failed\n");
 		return ret;
 	}
 
 	val &= ~REG_EMERGENCY_FORCE_HOST_CTRL;
 
-	ret = sx1301_write(spi, REG_EMERGENCY_FORCE, val);
+	ret = sx1301_write(priv, REG_EMERGENCY_FORCE, val);
 	if (ret) {
-		dev_err(&spi->dev, "emergency force write failed\n");
+		dev_err(priv->dev, "emergency force write failed\n");
 		return ret;
 	}
 
-	dev_err(&spi->dev, "starting calibration...\n");
+	dev_err(priv->dev, "starting calibration...\n");
 	msleep(2300);
 
-	ret = sx1301_read(spi, REG_EMERGENCY_FORCE, &val);
+	ret = sx1301_read(priv, REG_EMERGENCY_FORCE, &val);
 	if (ret) {
-		dev_err(&spi->dev, "emergency force read (1) failed\n");
+		dev_err(priv->dev, "emergency force read (1) failed\n");
 		return ret;
 	}
 
 	val |= REG_EMERGENCY_FORCE_HOST_CTRL;
 
-	ret = sx1301_write(spi, REG_EMERGENCY_FORCE, val);
+	ret = sx1301_write(priv, REG_EMERGENCY_FORCE, val);
 	if (ret) {
-		dev_err(&spi->dev, "emergency force write (1) failed\n");
+		dev_err(priv->dev, "emergency force write (1) failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(spi, REG_MCU_AGC_STATUS, &val);
+	ret = sx1301_read(priv, REG_MCU_AGC_STATUS, &val);
 	if (ret) {
-		dev_err(&spi->dev, "AGC status read failed\n");
+		dev_err(priv->dev, "AGC status read failed\n");
 		return ret;
 	}
 
-	dev_info(&spi->dev, "AGC status: %02x\n", (unsigned)val);
+	dev_info(priv->dev, "AGC status: %02x\n", (unsigned)val);
 	if ((val & (BIT(7) | BIT(0))) != (BIT(7) | BIT(0))) {
-		dev_err(&spi->dev, "AGC calibration failed\n");
+		dev_err(priv->dev, "AGC calibration failed\n");
 		return -ENXIO;
 	}
 
 	return 0;
 }
 
-static int sx1301_load_all_firmware(struct spi_device *spi)
+static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 {
 	const struct firmware *fw;
 	u8 val;
 	int ret;
 
-	ret = request_firmware(&fw, "sx1301_arb.bin", &spi->dev);
+	ret = request_firmware(&fw, "sx1301_arb.bin", priv->dev);
 	if (ret) {
-		dev_err(&spi->dev, "arb firmware file load failed\n");
+		dev_err(priv->dev, "arb firmware file load failed\n");
 		return ret;
 	}
 
 	if (fw->size != 8192) {
-		dev_err(&spi->dev, "unexpected arb firmware size\n");
+		dev_err(priv->dev, "unexpected arb firmware size\n");
 		release_firmware(fw);
 		return -EINVAL;
 	}
 
-	ret = sx1301_load_firmware(spi, 0, fw->data, fw->size);
+	ret = sx1301_load_firmware(priv, 0, fw->data, fw->size);
 	release_firmware(fw);
 	if (ret)
 		return ret;
 
-	ret = request_firmware(&fw, "sx1301_agc.bin", &spi->dev);
+	ret = request_firmware(&fw, "sx1301_agc.bin", priv->dev);
 	if (ret) {
-		dev_err(&spi->dev, "agc firmware file load failed\n");
+		dev_err(priv->dev, "agc firmware file load failed\n");
 		return ret;
 	}
 
 	if (fw->size != 8192) {
-		dev_err(&spi->dev, "unexpected agc firmware size\n");
+		dev_err(priv->dev, "unexpected agc firmware size\n");
 		release_firmware(fw);
 		return -EINVAL;
 	}
 
-	ret = sx1301_load_firmware(spi, 1, fw->data, fw->size);
+	ret = sx1301_load_firmware(priv, 1, fw->data, fw->size);
 	release_firmware(fw);
 	if (ret)
 		return ret;
 
-	ret = sx1301_page_read(spi, 0, 105, &val);
+	ret = sx1301_page_read(priv, 0, 105, &val);
 	if (ret) {
-		dev_err(&spi->dev, "0|105 read failed\n");
+		dev_err(priv->dev, "0|105 read failed\n");
 		return ret;
 	}
 
 	val &= ~(REG_0_105_FORCE_HOST_RADIO_CTRL | REG_0_105_FORCE_HOST_FE_CTRL | REG_0_105_FORCE_DEC_FILTER_GAIN);
 
-	ret = sx1301_page_write(spi, 0, 105, val);
+	ret = sx1301_page_write(priv, 0, 105, val);
 	if (ret) {
-		dev_err(&spi->dev, "0|105 write failed\n");
+		dev_err(priv->dev, "0|105 write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_write(spi, 0, REG_0_RADIO_SELECT, 0);
+	ret = sx1301_page_write(priv, 0, REG_0_RADIO_SELECT, 0);
 	if (ret) {
-		dev_err(&spi->dev, "radio select write failed\n");
+		dev_err(priv->dev, "radio select write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(spi, 0, REG_0_MCU, &val);
+	ret = sx1301_page_read(priv, 0, REG_0_MCU, &val);
 	if (ret) {
-		dev_err(&spi->dev, "MCU read (0) failed\n");
+		dev_err(priv->dev, "MCU read (0) failed\n");
 		return ret;
 	}
 
 	val &= ~(REG_0_MCU_RST_1 | REG_0_MCU_RST_0);
 
-	ret = sx1301_page_write(spi, 0, REG_0_MCU, val);
+	ret = sx1301_page_write(priv, 0, REG_0_MCU, val);
 	if (ret) {
-		dev_err(&spi->dev, "MCU write (0) failed\n");
+		dev_err(priv->dev, "MCU write (0) failed\n");
 		return ret;
 	}
 
-	ret = sx1301_agc_ram_read(spi, 0x20, &val);
+	ret = sx1301_agc_ram_read(priv, 0x20, &val);
 	if (ret) {
-		dev_err(&spi->dev, "AGC RAM data read failed\n");
+		dev_err(priv->dev, "AGC RAM data read failed\n");
 		return ret;
 	}
 
-	dev_info(&spi->dev, "AGC firmware version %u\n", (unsigned)val);
+	dev_info(priv->dev, "AGC firmware version %u\n", (unsigned)val);
 
 	if (val != 4) {
-		dev_err(&spi->dev, "unexpected firmware version, expecting %u\n", 4);
+		dev_err(priv->dev, "unexpected firmware version, expecting %u\n", 4);
 		return -ENXIO;
 	}
 
-	ret = sx1301_arb_ram_read(spi, 0x20, &val);
+	ret = sx1301_arb_ram_read(priv, 0x20, &val);
 	if (ret) {
-		dev_err(&spi->dev, "ARB RAM data read failed\n");
+		dev_err(priv->dev, "ARB RAM data read failed\n");
 		return ret;
 	}
 
-	dev_info(&spi->dev, "ARB firmware version %u\n", (unsigned)val);
+	dev_info(priv->dev, "ARB firmware version %u\n", (unsigned)val);
 
 	if (val != 1) {
-		dev_err(&spi->dev, "unexpected firmware version, expecting %u\n", 1);
+		dev_err(priv->dev, "unexpected firmware version, expecting %u\n", 1);
 		return -ENXIO;
 	}
 
@@ -635,17 +638,6 @@ static int sx1301_probe(struct spi_device *spi)
 	spi->bits_per_word = 8;
 	spi_setup(spi);
 
-	ret = sx1301_read(spi, REG_VERSION, &val);
-	if (ret) {
-		dev_err(&spi->dev, "version read failed\n");
-		return ret;
-	}
-
-	if (val != 103) {
-		dev_err(&spi->dev, "unexpected version: %u\n", val);
-		return -ENXIO;
-	}
-
 	netdev = devm_alloc_loradev(&spi->dev, sizeof(*priv));
 	if (!netdev)
 		return -ENOMEM;
@@ -654,22 +646,35 @@ static int sx1301_probe(struct spi_device *spi)
 	priv->rst_gpio = rst;
 	priv->cur_page = 0xff;
 
-	spi_set_drvdata(spi, netdev);
+	spi_set_drvdata(spi, priv);
+	priv->dev = &spi->dev;
+	priv->spi = spi;
 	SET_NETDEV_DEV(netdev, &spi->dev);
 
-	ret = sx1301_write(spi, REG_PAGE_RESET, 0);
+	ret = sx1301_read(priv, REG_VERSION, &val);
+	if (ret) {
+		dev_err(&spi->dev, "version read failed\n");
+		return ret;
+	}
+
+	if (val != 103) {
+		dev_err(&spi->dev, "unexpected version: %u\n", val);
+		return -ENXIO;
+	}
+
+	ret = sx1301_write(priv, REG_PAGE_RESET, 0);
 	if (ret) {
 		dev_err(&spi->dev, "page/reset write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_soft_reset(spi);
+	ret = sx1301_soft_reset(priv);
 	if (ret) {
 		dev_err(&spi->dev, "soft reset failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(spi, 16, &val);
+	ret = sx1301_read(priv, 16, &val);
 	if (ret) {
 		dev_err(&spi->dev, "16 read failed\n");
 		return ret;
@@ -677,13 +682,13 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val &= ~REG_16_GLOBAL_EN;
 
-	ret = sx1301_write(spi, 16, val);
+	ret = sx1301_write(priv, 16, val);
 	if (ret) {
 		dev_err(&spi->dev, "16 write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(spi, 17, &val);
+	ret = sx1301_read(priv, 17, &val);
 	if (ret) {
 		dev_err(&spi->dev, "17 read failed\n");
 		return ret;
@@ -691,13 +696,13 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val &= ~REG_17_CLK32M_EN;
 
-	ret = sx1301_write(spi, 17, val);
+	ret = sx1301_write(priv, 17, val);
 	if (ret) {
 		dev_err(&spi->dev, "17 write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_page_read(spi, 2, 43, &val);
+	ret = sx1301_page_read(priv, 2, 43, &val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 read failed\n");
 		return ret;
@@ -705,7 +710,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val |= REG_2_43_RADIO_B_EN | REG_2_43_RADIO_A_EN;
 
-	ret = sx1301_page_write(spi, 2, 43, val);
+	ret = sx1301_page_write(priv, 2, 43, val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 write failed\n");
 		return ret;
@@ -713,7 +718,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	msleep(500);
 
-	ret = sx1301_page_read(spi, 2, 43, &val);
+	ret = sx1301_page_read(priv, 2, 43, &val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 read failed\n");
 		return ret;
@@ -721,7 +726,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val |= REG_2_43_RADIO_RST;
 
-	ret = sx1301_page_write(spi, 2, 43, val);
+	ret = sx1301_page_write(priv, 2, 43, val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 write failed\n");
 		return ret;
@@ -729,7 +734,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	msleep(5);
 
-	ret = sx1301_page_read(spi, 2, 43, &val);
+	ret = sx1301_page_read(priv, 2, 43, &val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 read failed\n");
 		return ret;
@@ -737,7 +742,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val &= ~REG_2_43_RADIO_RST;
 
-	ret = sx1301_page_write(spi, 2, 43, val);
+	ret = sx1301_page_write(priv, 2, 43, val);
 	if (ret) {
 		dev_err(&spi->dev, "2|43 write failed\n");
 		return ret;
@@ -789,7 +794,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	/* GPIO */
 
-	ret = sx1301_read(spi, REG_GPIO_MODE, &val);
+	ret = sx1301_read(priv, REG_GPIO_MODE, &val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO mode read failed\n");
 		return ret;
@@ -797,13 +802,13 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val |= GENMASK(4, 0);
 
-	ret = sx1301_write(spi, REG_GPIO_MODE, val);
+	ret = sx1301_write(priv, REG_GPIO_MODE, val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO mode write failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(spi, REG_GPIO_SELECT_OUTPUT, &val);
+	ret = sx1301_read(priv, REG_GPIO_SELECT_OUTPUT, &val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO select output read failed\n");
 		return ret;
@@ -812,7 +817,7 @@ static int sx1301_probe(struct spi_device *spi)
 	val &= ~GENMASK(3, 0);
 	val |= 2;
 
-	ret = sx1301_write(spi, REG_GPIO_SELECT_OUTPUT, val);
+	ret = sx1301_write(priv, REG_GPIO_SELECT_OUTPUT, val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO select output write failed\n");
 		return ret;
@@ -820,7 +825,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	/* TODO LBT */
 
-	ret = sx1301_read(spi, 16, &val);
+	ret = sx1301_read(priv, 16, &val);
 	if (ret) {
 		dev_err(&spi->dev, "16 read (1) failed\n");
 		return ret;
@@ -828,13 +833,13 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val |= REG_16_GLOBAL_EN;
 
-	ret = sx1301_write(spi, 16, val);
+	ret = sx1301_write(priv, 16, val);
 	if (ret) {
 		dev_err(&spi->dev, "16 write (1) failed\n");
 		return ret;
 	}
 
-	ret = sx1301_read(spi, 17, &val);
+	ret = sx1301_read(priv, 17, &val);
 	if (ret) {
 		dev_err(&spi->dev, "17 read (1) failed\n");
 		return ret;
@@ -842,7 +847,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	val |= REG_17_CLK32M_EN;
 
-	ret = sx1301_write(spi, 17, val);
+	ret = sx1301_write(priv, 17, val);
 	if (ret) {
 		dev_err(&spi->dev, "17 write (1) failed\n");
 		return ret;
@@ -850,13 +855,13 @@ static int sx1301_probe(struct spi_device *spi)
 
 	/* calibration */
 
-	ret = sx1301_agc_calibrate(spi);
+	ret = sx1301_agc_calibrate(priv);
 	if (ret)
 		return ret;
 
 	/* TODO */
 
-	ret = sx1301_load_all_firmware(spi);
+	ret = sx1301_load_all_firmware(priv);
 	if (ret)
 		return ret;
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH lora-next v2 4/8] net: lora: sx1301: convert load_firmware to take firmware directly
From: Ben Whitten @ 2018-08-09 12:33 UTC (permalink / raw)
  To: afaerber; +Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten
In-Reply-To: <1533818018-29005-1-git-send-email-ben.whitten@lairdtech.com>

We just pass the pointer to firmware down to the function that loads
it.

Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
---
 drivers/net/lora/sx1301.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 7324001..3f2a532 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -287,13 +287,13 @@ static int sx1301_arb_ram_read(struct sx1301_priv *priv, u8 addr, u8 *val)
 	return 0;
 }
 
-static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const u8 *data, size_t len)
+static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const struct firmware *fw)
 {
 	u8 *buf;
 	u8 val, rst, select_mux;
 	int ret;
 
-	if (len > 8192)
+	if (fw->size > 8192)
 		return -EINVAL;
 
 	switch (mcu) {
@@ -330,7 +330,7 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const u8 *dat
 		return ret;
 	}
 
-	ret = sx1301_write_burst(priv, REG_MCU_PROM_DATA, data, len);
+	ret = sx1301_write_burst(priv, REG_MCU_PROM_DATA, fw->data, fw->size);
 	if (ret) {
 		dev_err(priv->dev, "MCU prom data write failed\n");
 		return ret;
@@ -342,18 +342,18 @@ static int sx1301_load_firmware(struct sx1301_priv *priv, int mcu, const u8 *dat
 		return ret;
 	}
 
-	buf = kzalloc(len, GFP_KERNEL);
+	buf = kzalloc(fw->size, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
-	ret = sx1301_read_burst(priv, REG_MCU_PROM_DATA, buf, len);
+	ret = sx1301_read_burst(priv, REG_MCU_PROM_DATA, buf, fw->size);
 	if (ret) {
 		dev_err(priv->dev, "MCU prom data read failed\n");
 		kfree(buf);
 		return ret;
 	}
 
-	if (memcmp(data, buf, len)) {
+	if (memcmp(fw->data, buf, fw->size)) {
 		dev_err(priv->dev, "MCU prom data read does not match data written\n");
 		kfree(buf);
 		return -ENXIO;
@@ -395,7 +395,7 @@ static int sx1301_agc_calibrate(struct sx1301_priv *priv)
 		return -EINVAL;
 	}
 
-	ret = sx1301_load_firmware(priv, 1, fw->data, fw->size);
+	ret = sx1301_load_firmware(priv, 1, fw);
 	release_firmware(fw);
 	if (ret) {
 		dev_err(priv->dev, "agc cal firmware load failed\n");
@@ -523,7 +523,7 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 		return -EINVAL;
 	}
 
-	ret = sx1301_load_firmware(priv, 0, fw->data, fw->size);
+	ret = sx1301_load_firmware(priv, 0, fw);
 	release_firmware(fw);
 	if (ret)
 		return ret;
@@ -540,7 +540,7 @@ static int sx1301_load_all_firmware(struct sx1301_priv *priv)
 		return -EINVAL;
 	}
 
-	ret = sx1301_load_firmware(priv, 1, fw->data, fw->size);
+	ret = sx1301_load_firmware(priv, 1, fw);
 	release_firmware(fw);
 	if (ret)
 		return ret;
-- 
2.7.4

^ permalink raw reply related

* [PATCH lora-next v2 2/8] net: lora: sx1301: convert to devm registration of netdev
From: Ben Whitten @ 2018-08-09 12:33 UTC (permalink / raw)
  To: afaerber; +Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten
In-Reply-To: <1533818018-29005-1-git-send-email-ben.whitten@lairdtech.com>

We allow the devres framework handle the clean removal of resources on
teardown of the device, in this case the SPI device, saving lengthy
unwind code and improving clarity.

Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
---
 drivers/net/lora/sx1301.c | 87 +++++++++++++++--------------------------------
 1 file changed, 27 insertions(+), 60 deletions(-)

diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 5342b61..3c09f5a 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -3,6 +3,7 @@
  * Semtech SX1301 LoRa concentrator
  *
  * Copyright (c) 2018 Andreas Färber
+ * Copyright (c) 2018 Ben Whitten
  *
  * Based on SX1301 HAL code:
  * Copyright (c) 2013 Semtech-Cycleo
@@ -637,20 +638,17 @@ static int sx1301_probe(struct spi_device *spi)
 	ret = sx1301_read(spi, REG_VERSION, &val);
 	if (ret) {
 		dev_err(&spi->dev, "version read failed\n");
-		goto err_version;
+		return ret;
 	}
 
 	if (val != 103) {
 		dev_err(&spi->dev, "unexpected version: %u\n", val);
-		ret = -ENXIO;
-		goto err_version;
+		return -ENXIO;
 	}
 
-	netdev = alloc_loradev(sizeof(*priv));
-	if (!netdev) {
-		ret = -ENOMEM;
-		goto err_alloc_loradev;
-	}
+	netdev = devm_alloc_loradev(&spi->dev, sizeof(*priv));
+	if (!netdev)
+		return -ENOMEM;
 
 	priv = netdev_priv(netdev);
 	priv->rst_gpio = rst;
@@ -662,19 +660,19 @@ static int sx1301_probe(struct spi_device *spi)
 	ret = sx1301_write(spi, REG_PAGE_RESET, 0);
 	if (ret) {
 		dev_err(&spi->dev, "page/reset write failed\n");
-		goto err_init_page;
+		return ret;
 	}
 
 	ret = sx1301_soft_reset(spi);
 	if (ret) {
 		dev_err(&spi->dev, "soft reset failed\n");
-		goto err_soft_reset;
+		return ret;
 	}
 
 	ret = sx1301_read(spi, 16, &val);
 	if (ret) {
 		dev_err(&spi->dev, "16 read failed\n");
-		goto err_read_global_en_0;
+		return ret;
 	}
 
 	val &= ~REG_16_GLOBAL_EN;
@@ -682,13 +680,13 @@ static int sx1301_probe(struct spi_device *spi)
 	ret = sx1301_write(spi, 16, val);
 	if (ret) {
 		dev_err(&spi->dev, "16 write failed\n");
-		goto err_write_global_en_0;
+		return ret;
 	}
 
 	ret = sx1301_read(spi, 17, &val);
 	if (ret) {
 		dev_err(&spi->dev, "17 read failed\n");
-		goto err_read_clk32m_0;
+		return ret;
 	}
 
 	val &= ~REG_17_CLK32M_EN;
@@ -696,7 +694,7 @@ static int sx1301_probe(struct spi_device *spi)
 	ret = sx1301_write(spi, 17, val);
 	if (ret) {
 		dev_err(&spi->dev, "17 write failed\n");
-		goto err_write_clk32m_0;
+		return ret;
 	}
 
 	ret = sx1301_page_read(spi, 2, 43, &val);
@@ -749,8 +747,7 @@ static int sx1301_probe(struct spi_device *spi)
 
 	priv->radio_a_ctrl = spi_alloc_master(&spi->dev, sizeof(*radio));
 	if (!priv->radio_a_ctrl) {
-		ret = -ENOMEM;
-		goto err_radio_a_alloc;
+		return -ENOMEM;
 	}
 
 	sx1301_radio_setup(priv->radio_a_ctrl);
@@ -765,15 +762,14 @@ static int sx1301_probe(struct spi_device *spi)
 	if (ret) {
 		dev_err(&spi->dev, "radio A SPI register failed\n");
 		spi_controller_put(priv->radio_a_ctrl);
-		goto err_radio_a_register;
+		return ret;
 	}
 
 	/* radio B */
 
 	priv->radio_b_ctrl = spi_alloc_master(&spi->dev, sizeof(*radio));
 	if (!priv->radio_b_ctrl) {
-		ret = -ENOMEM;
-		goto err_radio_b_alloc;
+		return -ENOMEM;
 	}
 
 	sx1301_radio_setup(priv->radio_b_ctrl);
@@ -788,7 +784,7 @@ static int sx1301_probe(struct spi_device *spi)
 	if (ret) {
 		dev_err(&spi->dev, "radio B SPI register failed\n");
 		spi_controller_put(priv->radio_b_ctrl);
-		goto err_radio_b_register;
+		return ret;
 	}
 
 	/* GPIO */
@@ -796,7 +792,7 @@ static int sx1301_probe(struct spi_device *spi)
 	ret = sx1301_read(spi, REG_GPIO_MODE, &val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO mode read failed\n");
-		goto err_read_gpio_mode;
+		return ret;
 	}
 
 	val |= GENMASK(4, 0);
@@ -804,13 +800,13 @@ static int sx1301_probe(struct spi_device *spi)
 	ret = sx1301_write(spi, REG_GPIO_MODE, val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO mode write failed\n");
-		goto err_write_gpio_mode;
+		return ret;
 	}
 
 	ret = sx1301_read(spi, REG_GPIO_SELECT_OUTPUT, &val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO select output read failed\n");
-		goto err_read_gpio_select_output;
+		return ret;
 	}
 
 	val &= ~GENMASK(3, 0);
@@ -819,7 +815,7 @@ static int sx1301_probe(struct spi_device *spi)
 	ret = sx1301_write(spi, REG_GPIO_SELECT_OUTPUT, val);
 	if (ret) {
 		dev_err(&spi->dev, "GPIO select output write failed\n");
-		goto err_write_gpio_select_output;
+		return ret;
 	}
 
 	/* TODO LBT */
@@ -827,7 +823,7 @@ static int sx1301_probe(struct spi_device *spi)
 	ret = sx1301_read(spi, 16, &val);
 	if (ret) {
 		dev_err(&spi->dev, "16 read (1) failed\n");
-		goto err_read_global_en_1;
+		return ret;
 	}
 
 	val |= REG_16_GLOBAL_EN;
@@ -835,13 +831,13 @@ static int sx1301_probe(struct spi_device *spi)
 	ret = sx1301_write(spi, 16, val);
 	if (ret) {
 		dev_err(&spi->dev, "16 write (1) failed\n");
-		goto err_write_global_en_1;
+		return ret;
 	}
 
 	ret = sx1301_read(spi, 17, &val);
 	if (ret) {
 		dev_err(&spi->dev, "17 read (1) failed\n");
-		goto err_read_clk32m_1;
+		return ret;
 	}
 
 	val |= REG_17_CLK32M_EN;
@@ -849,58 +845,28 @@ static int sx1301_probe(struct spi_device *spi)
 	ret = sx1301_write(spi, 17, val);
 	if (ret) {
 		dev_err(&spi->dev, "17 write (1) failed\n");
-		goto err_write_clk32m_1;
+		return ret;
 	}
 
 	/* calibration */
 
 	ret = sx1301_agc_calibrate(spi);
 	if (ret)
-		goto err_agc_calibrate;
+		return ret;
 
 	/* TODO */
 
 	ret = sx1301_load_all_firmware(spi);
 	if (ret)
-		goto err_load_firmware;
+		return ret;
 
 	dev_info(&spi->dev, "SX1301 module probed\n");
 
 	return 0;
-
-err_load_firmware:
-err_agc_calibrate:
-err_write_clk32m_1:
-err_read_clk32m_1:
-err_write_global_en_1:
-err_read_global_en_1:
-err_write_gpio_select_output:
-err_read_gpio_select_output:
-err_write_gpio_mode:
-err_read_gpio_mode:
-err_radio_b_register:
-err_radio_b_alloc:
-err_radio_a_register:
-err_radio_a_alloc:
-err_write_clk32m_0:
-err_read_clk32m_0:
-err_write_global_en_0:
-err_read_global_en_0:
-err_soft_reset:
-err_init_page:
-	free_loradev(netdev);
-err_alloc_loradev:
-err_version:
-	return ret;
 }
 
 static int sx1301_remove(struct spi_device *spi)
 {
-	struct net_device *netdev = spi_get_drvdata(spi);
-
-	//unregister_loradev(netdev);
-	free_loradev(netdev);
-
 	dev_info(&spi->dev, "SX1301 module removed\n");
 
 	return 0;
@@ -927,4 +893,5 @@ module_spi_driver(sx1301_spi_driver);
 
 MODULE_DESCRIPTION("SX1301 SPI driver");
 MODULE_AUTHOR("Andreas Färber <afaerber@suse.de>");
+MODULE_AUTHOR("Ben Whitten <ben.whitten@gmail.com>");
 MODULE_LICENSE("GPL");
-- 
2.7.4

^ permalink raw reply related

* [PATCH lora-next v2 1/8] net: lora: add methods for devm registration
From: Ben Whitten @ 2018-08-09 12:33 UTC (permalink / raw)
  To: afaerber; +Cc: starnight, hasnain.virk, netdev, liuxuenetmail, shess,
	Ben Whitten

Follow the devm model so that we can avoid lengthy unwind code.

Signed-off-by: Ben Whitten <ben.whitten@lairdtech.com>
---
 drivers/net/lora/dev.c   | 28 ++++++++++++++++++++++++++++
 include/linux/lora/dev.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/net/lora/dev.c b/drivers/net/lora/dev.c
index 8c01106..e32a870 100644
--- a/drivers/net/lora/dev.c
+++ b/drivers/net/lora/dev.c
@@ -84,6 +84,34 @@ void free_loradev(struct net_device *dev)
 }
 EXPORT_SYMBOL_GPL(free_loradev);
 
+static void devm_free_loradev(struct device *dev, void *res)
+{
+	struct net_device *net = (*(struct net_device **)res);
+	free_loradev(net);
+}
+
+struct net_device *devm_alloc_loradev(struct device *dev, size_t priv)
+{
+	struct net_device **ptr;
+	struct net_device *net;
+
+	net = alloc_loradev(priv);
+	if (!net)
+		return NULL;
+
+	ptr = devres_alloc(devm_free_loradev, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr) {
+		free_loradev(net);
+		return NULL;
+	}
+
+	*ptr = net;
+	devres_add(dev, ptr);
+
+	return net;
+}
+EXPORT_SYMBOL_GPL(devm_alloc_loradev);
+
 static struct rtnl_link_ops lora_link_ops __read_mostly = {
 	.kind = "lora",
 	.setup = lora_setup,
diff --git a/include/linux/lora/dev.h b/include/linux/lora/dev.h
index 153f9b2..0f600c9 100644
--- a/include/linux/lora/dev.h
+++ b/include/linux/lora/dev.h
@@ -31,6 +31,7 @@ static inline int lora_strtoeui(const char *str, lora_eui *val)
 }
 
 struct net_device *alloc_loradev(int sizeof_priv);
+struct net_device *devm_alloc_loradev(struct device *dev, size_t priv);
 void free_loradev(struct net_device *dev);
 int register_loradev(struct net_device *dev);
 void unregister_loradev(struct net_device *dev);
-- 
2.7.4

^ permalink raw reply related

* Re: [Xen-devel] [PATCH] xen/netfront: don't cache skb_shinfo()
From: Wei Liu @ 2018-08-09 14:47 UTC (permalink / raw)
  To: Juergen Gross
  Cc: linux-kernel, xen-devel, netdev, boris.ostrovsky, davem, stable,
	Wei Liu
In-Reply-To: <20180809144216.18856-1-jgross@suse.com>

On Thu, Aug 09, 2018 at 04:42:16PM +0200, Juergen Gross wrote:
> skb_shinfo() can change when calling __pskb_pull_tail(): Don't cache
> its return value.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

^ permalink raw reply

* [PATCH] xen/netfront: don't cache skb_shinfo()
From: Juergen Gross @ 2018-08-09 14:42 UTC (permalink / raw)
  To: linux-kernel, xen-devel, netdev
  Cc: boris.ostrovsky, davem, Juergen Gross, stable

skb_shinfo() can change when calling __pskb_pull_tail(): Don't cache
its return value.

Cc: stable@vger.kernel.org
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/net/xen-netfront.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 2d8812dd1534..9dd2ca62d84a 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -894,7 +894,6 @@ static RING_IDX xennet_fill_frags(struct netfront_queue *queue,
 				  struct sk_buff *skb,
 				  struct sk_buff_head *list)
 {
-	struct skb_shared_info *shinfo = skb_shinfo(skb);
 	RING_IDX cons = queue->rx.rsp_cons;
 	struct sk_buff *nskb;
 
@@ -903,15 +902,16 @@ static RING_IDX xennet_fill_frags(struct netfront_queue *queue,
 			RING_GET_RESPONSE(&queue->rx, ++cons);
 		skb_frag_t *nfrag = &skb_shinfo(nskb)->frags[0];
 
-		if (shinfo->nr_frags == MAX_SKB_FRAGS) {
+		if (skb_shinfo(skb)->nr_frags == MAX_SKB_FRAGS) {
 			unsigned int pull_to = NETFRONT_SKB_CB(skb)->pull_to;
 
 			BUG_ON(pull_to <= skb_headlen(skb));
 			__pskb_pull_tail(skb, pull_to - skb_headlen(skb));
 		}
-		BUG_ON(shinfo->nr_frags >= MAX_SKB_FRAGS);
+		BUG_ON(skb_shinfo(skb)->nr_frags >= MAX_SKB_FRAGS);
 
-		skb_add_rx_frag(skb, shinfo->nr_frags, skb_frag_page(nfrag),
+		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+				skb_frag_page(nfrag),
 				rx->offset, rx->status, PAGE_SIZE);
 
 		skb_shinfo(nskb)->nr_frags = 0;
-- 
2.13.7

^ permalink raw reply related

* Re: [PATCH v1 6/7] net: mvneta: Don't use GRO on Armada 3720
From: Jisheng Zhang @ 2018-08-09 12:08 UTC (permalink / raw)
  To: Marek Behún, Thomas Petazzoni, Andrew Lunn
  Cc: linux-arm-kernel, netdev, Gregory CLEMENT, Tomas Hlavacek,
	Russell King - ARM Linux, David S. Miller
In-Reply-To: <20180809192755.24e57782@xhacker.debian>

On Thu, 9 Aug 2018 19:27:55 +0800 Jisheng Zhang wrote:

> Hi,
> 
> On Thu, 9 Aug 2018 12:40:41 +0800 Jisheng Zhang wrote:
> 
> > + more people
> > 
> > On Wed,  8 Aug 2018 17:27:05 +0200 Marek Behún wrote:
> >   
> > > For some reason on Armada 3720 boards (EspressoBin and Turris Mox) the
> > > networking driver behaves weirdly when using napi_gro_receive.
> > > 
> > > For example downloading a big file from a local network (low ping) is
> > > fast, but when downloading from a remote server (higher ping), the
> > > download speed is at first high but drops rapidly to almost nothing or
> > > absolutely nothing.    
> > 
> > We also met this issue on some berlin platforms. I tried to fix the bug,
> > but no clue so far.
> >   
> > > 
> > > This is fixed when using netif_receive_skb instead of napi_gro_receive.    
> > 
> > This is a workaround. The good news is this workaround also fixes the issue
> > we saw on berlin.
> >   
> > > 
> > > Signed-off-by: Marek Behun <marek.behun@nic.cz>
> > > Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> > > Cc: netdev@vger.kernel.org
> > > 
> > > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > > index 0ad2f3f7da85..27f3017d94c5 100644
> > > --- a/drivers/net/ethernet/marvell/mvneta.c
> > > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > > @@ -1959,7 +1959,10 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> > >  
> > >  			skb->protocol = eth_type_trans(skb, dev);
> > >  			mvneta_rx_csum(pp, rx_status, skb);
> > > -			napi_gro_receive(&port->napi, skb);
> > > +			if (pp->neta_armada3700)
> > > +				netif_receive_skb(skb);
> > > +			else
> > > +				napi_gro_receive(&port->napi, skb);  
> 
> I think I found the root cause, if neta_armada3700 is true, the port got from
> this_cpu_ptr(pp->ports) is invalid, this is bug... I'll cook a patch for this

correct it as:

the port's(port is got from this_cpu_ptr(pp->ports) napi is invalid.

Patch is sent out. Could you please try?

Per my test, it solves the issue we saw on berlin.

> 
> Thanks
> 
> > >  
> > >  			rcvd_pkts++;
> > >  			rcvd_bytes += rx_bytes;
> > > @@ -2001,7 +2004,10 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> > >  
> > >  		mvneta_rx_csum(pp, rx_status, skb);
> > >  
> > > -		napi_gro_receive(&port->napi, skb);
> > > +		if (pp->neta_armada3700)
> > > +			netif_receive_skb(skb);
> > > +		else
> > > +			napi_gro_receive(&port->napi, skb);
> > >  	}
> > >  
> > >  	if (rcvd_pkts) {    
> >   
> 

^ permalink raw reply

* Re: [Query]: DSA Understanding
From: Andrew Lunn @ 2018-08-09 12:08 UTC (permalink / raw)
  To: Lad, Prabhakar; +Cc: Florian Fainelli, netdev
In-Reply-To: <CA+V-a8s9z2=aXiXC-82_sGwsZwKW6Sk6F3=LDX8Uz6B0L_VUMw@mail.gmail.com>

On Thu, Aug 09, 2018 at 12:33:30PM +0100, Lad, Prabhakar wrote:
> Hi Florain,
> 
> Thanks for your reply.
> 
> On Thu, Aug 2, 2018 at 5:24 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >
> >
> >
> > On 08/02/2018 09:05 AM, Andrew Lunn wrote:
> > >> I dont see any Reply's on the PC with tcpdump on PC
> > >
> > > So try ethool -S on the PC. Any packets dropped because of errors?
> > >
> > > Try turning off hardware checksums on the switch. ethtool -K.
> >
> > Also make sure that cpsw is properly sending 64 bytes (including FCS)
> > packets to the switch, some switches will just discard packets when
> > packets are RUNT
> 
> how can I dump the FCS from the switch (can I use tshark ?)

tskark running on the target will not show you the FCS. The Ethernet
hardware should calculate that as it sends the packet. Wireshark on
the PC might, but some Ethernet cards strip off the FCS before passing
it to the stack.

What is important here is the idea of a runt packet. You appear to be
receiving some packets at the PC, if we can trust the statistics you
showed. Are those received packets all big? Are the small ARP replies
missing?

	Andrew

^ permalink raw reply

* Re: [Query]: DSA Understanding
From: Andrew Lunn @ 2018-08-09 12:01 UTC (permalink / raw)
  To: Lad, Prabhakar; +Cc: netdev
In-Reply-To: <CA+V-a8u0W4JA_HrorXZApFbAOBHiWdVpVK0DvSmcKftr5NrC8w@mail.gmail.com>

On Thu, Aug 09, 2018 at 12:31:31PM +0100, Lad, Prabhakar wrote:
> Hi Andrew,
> 
> On Thu, Aug 2, 2018 at 5:05 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > I dont see any Reply's on the PC with tcpdump on PC
> >
> > So try ethool -S on the PC. Any packets dropped because of errors?
> >
> I dont see any drops/errors on the PC, following is the dump from PC:
> 
> sudo ethtool -S enx00e04c68c229
> [sudo] password for prabhakar:
> NIC statistics:
>      tx_packets: 1659
>      rx_packets: 485
>      tx_errors: 0
>      rx_errors: 0
>      rx_missed: 0
>      align_errors: 0
>      tx_single_collisions: 0
>      tx_multi_collisions: 0
>      rx_unicast: 18
>      rx_broadcast: 295
>      rx_multicast: 172
>      tx_aborted: 0
>      tx_underrun: 0

So there are received packets at the PC. Not many unicast, mostly
broadcast, which fits with ARP. What does wireshark tell you about
these received packets? Are they ARP replies? Are they something else?
If they are ARP replies, why are they being ignored?  I don't know if
tshark will show you CRC problems. Wireshark does, when you unfold a
packet, and look at the fields in detail.

> Seems like the packet is not being transmitted from the switch at all
> ? (as ping from switch lan4 to PC fails)

I don't think you can make that conclusion yet. The PC is receiving
something, rx_packets=485. What are those packets?

Look at the statistics along the chain, from the target to the PC.
Look at the master device, lan4 and the PC. You should see about one
packet per second transmitted on the master device. One packet per
second transmitted on lan4, and one packet per second received on the
PC. Where does this break down.

	   Andrew

^ permalink raw reply

* Re: [PATCH] netfilter: nf_queue: Replace conntrack entry
From: Andrey Jr. Melnikov @ 2018-08-09 14:09 UTC (permalink / raw)
  To: linux-kernel; +Cc: netfilter-devel, netdev
In-Reply-To: <20180503140745.26588-1-kristian.evensen@gmail.com>

In gmane.comp.security.firewalls.netfilter.devel Kristian Evensen <kristian.evensen@gmail.com> wrote:
> SKBs are assigned a conntrack entry before being passed to any NFQUEUEs,
> and if no entry is found then a new one is created. This behavior causes
> problems for some traffic patterns. For example, if two UDP packets
> to/from the same host (using the same ports) arrive at the "same" time,
> both are assigned a new conntrack entry. After the first packet have
> traversed all chains, the conntrack entry will be inserted into the
> global table. The second packet will then be dropped during the
> insertion step, as an entry for the same flow already exists. One type
> of application that frequently generates this traffic pattern, is DNS
> resolvers.

> This commit introduces a new function that checks, and potentially
> replaces, the conntrack entry for any additional "new" SKBs mapping to
> an existing flow. While not a perfect solution, there are still
> situations where to-be-dropped SKBs can slip through, the situations is
> improved considerably. On the routers I have used for testing, packets
> belonging to the same UDP flow are let through (when generating the
> traffic pattern described above). Without the change in this commit, all
> packets except the first one was dropped.

> With the change in this commit, a user can implement "perfect" solutions
> in user-space. An application can for example keep track of seen UDP
> flows, and then only release packets belonging to one flow when the
> entry has been created. Without the change, and SKB is stuck with the
> original conntrack entry.

PING
Any progress on this patch?

> Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
> ---
>  net/netfilter/nfnetlink_queue.c | 68 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)

[...]

^ permalink raw reply

* Re: [PATCH v6 8/9] net/mlx5: Do not call pcie_print_link_status()
From: Bjorn Helgaas @ 2018-08-09 14:02 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Alex G., Tal Gilboa, linux-pci, bhelgaas, jakub.kicinski,
	keith.busch, alex_gagniuc, austin_bolen, shyam_iyer, Ariel Elior,
	everest-linux-l2, David S. Miller, Michael Chan, Ganesh Goudar,
	Jeff Kirsher, Tariq Toukan, Saeed Mahameed, Dirk van der Merwe,
	netdev, linux-kernel, intel-wired-lan, linux-rdma, oss-drivers
In-Reply-To: <20180808172736.GB13378@mtr-leonro.mtl.com>

On Wed, Aug 08, 2018 at 08:27:36PM +0300, Leon Romanovsky wrote:
> On Wed, Aug 08, 2018 at 11:33:51AM -0500, Alex G. wrote:
> >
> >
> > On 08/08/2018 10:56 AM, Tal Gilboa wrote:
> > > On 8/8/2018 6:41 PM, Leon Romanovsky wrote:
> > > > On Wed, Aug 08, 2018 at 05:23:12PM +0300, Tal Gilboa wrote:
> > > > > On 8/8/2018 9:08 AM, Leon Romanovsky wrote:
> > > > > > On Mon, Aug 06, 2018 at 06:25:42PM -0500, Alexandru Gagniuc wrote:
> > > > > > > This is now done by the PCI core to warn of sub-optimal bandwidth.
> > > > > > >
> > > > > > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > > > > > > ---
> > > > > > >    drivers/net/ethernet/mellanox/mlx5/core/main.c | 4 ----
> > > > > > >    1 file changed, 4 deletions(-)
> > > > > > >
> > > > > >
> > > > > > Thanks,
> > > > > > Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
> > > > > >
> > > > >
> > > > > Alex,
> > > > > I loaded mlx5 driver with and without these series. The report
> > > > > in dmesg is
> > > > > now missing. From what I understood, the status should be
> > > > > reported at least
> > > > > once, even if everything is in order.
> > > >
> > > > It is not what this series is doing and it removes prints completely if
> > > > fabric can deliver more than card is capable.
> > > >
> > > > > We need this functionality to stay.
> > > >
> > > > I'm not sure that you need this information in driver's dmesg output,
> > > > but most probably something globally visible and accessible per-pci
> > > > device.
> > >
> > > Currently we have users that look for it. If we remove the dmesg print
> > > we need this to be reported elsewhere. Adding it to sysfs for example
> > > should be a valid solution for our case.
> >
> > I think a stop-gap measure is to leave the pcie_print_link_status() call in
> > drivers that really need it for whatever reason. Implementing a reliable
> > reporting through sysfs might take some tinkering, and I don't think it's a
> > sufficient reason to block the heart of this series -- being able to detect
> > bottlenecks and link downtraining.
> 
> IMHO, you did right change and it is better to replace this print to some
> more generic solution now while you are doing it and don't leave leftovers.

I'd like to make forward progress on this, so I propose we merge only
the PCI core change (patch 1/9) and drop the individual driver
changes.  That would mean:

  - We'll get a message from every NIC driver that calls
    pcie_print_link_status() as before.

  - We'll get a new message from the core for every downtrained link.

  - If a link leading to the NIC is downtrained, there will be
    duplicate messages.  Maybe that's overkill but it's not terrible.

I provisionally put the patch below on my pci/enumeration branch.
Objections?


commit c870cc8cbc4d79014f3daa74d1e412f32e42bf1b
Author: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Date:   Mon Aug 6 18:25:35 2018 -0500

    PCI: Check for PCIe Link downtraining
    
    When both ends of a PCIe Link are capable of a higher bandwidth than is
    currently in use, the Link is said to be "downtrained".  A downtrained Link
    may indicate hardware or configuration problems in the system, but it's
    hard to identify such Links from userspace.
    
    Refactor pcie_print_link_status() so it continues to always print PCIe
    bandwidth information, as several NIC drivers desire.
    
    Add a new internal __pcie_print_link_status() to emit a message only when a
    device's bandwidth is constrained by the fabric and call it from the PCI
    core for all devices, which identifies all downtrained Links.  It also
    emits messages for a few cases that are technically not downtrained, such
    as a x4 device in an open-ended x1 slot.
    
    Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
    [bhelgaas: changelog, move __pcie_print_link_status() declaration to
    drivers/pci/, rename pcie_check_upstream_link() to
    pcie_report_downtraining()]
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 97acba712e4e..a84d341504a5 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5264,14 +5264,16 @@ u32 pcie_bandwidth_capable(struct pci_dev *dev, enum pci_bus_speed *speed,
 }
 
 /**
- * pcie_print_link_status - Report the PCI device's link speed and width
+ * __pcie_print_link_status - Report the PCI device's link speed and width
  * @dev: PCI device to query
+ * @verbose: Print info even when enough bandwidth is available
  *
- * Report the available bandwidth at the device.  If this is less than the
- * device is capable of, report the device's maximum possible bandwidth and
- * the upstream link that limits its performance to less than that.
+ * If the available bandwidth at the device is less than the device is
+ * capable of, report the device's maximum possible bandwidth and the
+ * upstream link that limits its performance.  If @verbose, always print
+ * the available bandwidth, even if the device isn't constrained.
  */
-void pcie_print_link_status(struct pci_dev *dev)
+void __pcie_print_link_status(struct pci_dev *dev, bool verbose)
 {
 	enum pcie_link_width width, width_cap;
 	enum pci_bus_speed speed, speed_cap;
@@ -5281,11 +5283,11 @@ void pcie_print_link_status(struct pci_dev *dev)
 	bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap);
 	bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width);
 
-	if (bw_avail >= bw_cap)
+	if (bw_avail >= bw_cap && verbose)
 		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)\n",
 			 bw_cap / 1000, bw_cap % 1000,
 			 PCIE_SPEED2STR(speed_cap), width_cap);
-	else
+	else if (bw_avail < bw_cap)
 		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)\n",
 			 bw_avail / 1000, bw_avail % 1000,
 			 PCIE_SPEED2STR(speed), width,
@@ -5293,6 +5295,17 @@ void pcie_print_link_status(struct pci_dev *dev)
 			 bw_cap / 1000, bw_cap % 1000,
 			 PCIE_SPEED2STR(speed_cap), width_cap);
 }
+
+/**
+ * pcie_print_link_status - Report the PCI device's link speed and width
+ * @dev: PCI device to query
+ *
+ * Report the available bandwidth at the device.
+ */
+void pcie_print_link_status(struct pci_dev *dev)
+{
+	__pcie_print_link_status(dev, true);
+}
 EXPORT_SYMBOL(pcie_print_link_status);
 
 /**
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 70808c168fb9..ce880dab5bc8 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -263,6 +263,7 @@ enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev);
 enum pcie_link_width pcie_get_width_cap(struct pci_dev *dev);
 u32 pcie_bandwidth_capable(struct pci_dev *dev, enum pci_bus_speed *speed,
 			   enum pcie_link_width *width);
+void __pcie_print_link_status(struct pci_dev *dev, bool verbose);
 
 /* Single Root I/O Virtualization */
 struct pci_sriov {
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index bc147c586643..387fc8ac54ec 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2231,6 +2231,25 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	return dev;
 }
 
+static void pcie_report_downtraining(struct pci_dev *dev)
+{
+	if (!pci_is_pcie(dev))
+		return;
+
+	/* Look from the device up to avoid downstream ports with no devices */
+	if ((pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT) &&
+	    (pci_pcie_type(dev) != PCI_EXP_TYPE_LEG_END) &&
+	    (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM))
+		return;
+
+	/* Multi-function PCIe devices share the same link/status */
+	if (PCI_FUNC(dev->devfn) != 0 || dev->is_virtfn)
+		return;
+
+	/* Print link status only if the device is constrained by the fabric */
+	__pcie_print_link_status(dev, false);
+}
+
 static void pci_init_capabilities(struct pci_dev *dev)
 {
 	/* Enhanced Allocation */
@@ -2266,6 +2285,8 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	/* Advanced Error Reporting */
 	pci_aer_init(dev);
 
+	pcie_report_downtraining(dev);
+
 	if (pci_probe_reset_function(dev) == 0)
 		dev->reset_fn = 1;
 }

^ permalink raw reply related

* Re: [Query]: DSA Understanding
From: Lad, Prabhakar @ 2018-08-09 11:33 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Andrew Lunn, netdev
In-Reply-To: <b87d9026-3aed-1bcd-835e-608a29bf31e2@gmail.com>

Hi Florain,

Thanks for your reply.

On Thu, Aug 2, 2018 at 5:24 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 08/02/2018 09:05 AM, Andrew Lunn wrote:
> >> I dont see any Reply's on the PC with tcpdump on PC
> >
> > So try ethool -S on the PC. Any packets dropped because of errors?
> >
> > Try turning off hardware checksums on the switch. ethtool -K.
>
> Also make sure that cpsw is properly sending 64 bytes (including FCS)
> packets to the switch, some switches will just discard packets when
> packets are RUNT

how can I dump the FCS from the switch (can I use tshark ?)

Cheers,
--Prabhakar Lad

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox