Re: [GIT] Networking
Hi all,
> On Wed, Sep 2, 2015 at 10:35 PM, David Miller wrote:
>>
>> Another merge window, another set of networking changes. I've heard
>> rumblings that the lightweight tunnels infrastructure has been voted
>> networking change of the year.
>
> .. and others say that the most notable feature is the idiotic bugs
> that it introduces, and the compiler even complains about.
>
> Christ, people. Learn C, instead of just stringing random characters
> together until it compiles (with warnings).
>
> This:
>
> static bool rate_control_cap_mask(struct ieee80211_sub_if_data *sdata,
>struct ieee80211_supported_band *sband,
>struct ieee80211_sta *sta, u32 *mask,
>u8 mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
>
> is horribly broken to begin with, because array arguments in C don't
> actually exist. Sadly, compilers accept it for various bad historical
> reasons, and silently turn it into just a pointer argument. There are
> arguments for them, but they are from weak minds.
>
> But happily gcc has a really really valid warning (kudos - I often end
> up ragging on the bad warnings gcc has, but this one is a keeper),
> because a few lines down the mistake then turns into pure and utter
> garbage.
>
> It's garbage that was basically encouraged by the first mistake
> (thinking that C allows array arguments), namely:
>
> for (i = 0; i < sizeof(mcs_mask); i++)
>
I moved rate_control_apply_mask() logic in rate_control_cap_mask() in
order to be applied in multiple code points (i.e.
rate_control_apply_mask_ratetbl()). Since I was using gcc version
4.9.2, the warning did not show up. Sorry for that bug.
> the "sizeof(mcs_mask)" is _shit_. Since array arguments don't actually
> exist in C, it is the size of the pointer, not the array. The first
> mistake makes the bug look like reasonable code. Although I'd argue
> that the code would actually be bad regardless, since "sizeof" is the
> size in bytes, and the code actually wants the number of entries (and
> we do have ARRAY_SIZE() for that).
>
> Sure, in this case the entries are just one byte each, so it would
> have *worked* had it not been for the array argument issue, but it's
> misleading and the code is just fundamentally buggy and nonsensical in
> two entirely different ways that fed on each other.
>
> That line should read
>
> for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++)
>
> and the argument should just have been declared as the pointer it actually is.
>
> A later patch then added onto the pile of manure by adding *another*
> broken array argument, but at least that one then used the proper loop
> for traversal of that array.
>
> The fact that I notice this bug from a very basic "let's just compile
> each pull request and make sure it isn't complete crap" is sad.
>
> Now, it *looks* like the code was just moved, and the "sizeof()" was
> initially correct (because it was a size of an actual array). Well, it
> was "correct" in the sense that it generated the right code, even if
> the whole confusion between "number of entries" and "size in bytes"
> was still there. Then it got moved and turned from "confused but
> happens to generate correct code" into "buggy pile of bovine manure".
> See commit 90c66bd2232a ("mac80211: remove ieee80211_tx_rate
> dependency in rate mask code").
>
> So I can see how this bug happened, and I am only slightly upset with
> Lorenzo who is the author of that commit.
>
> What I can't see is why the code has existed in at least two
> maintainer trees (Johannes' and David's) for a couple of weeks, and
> nobody cared about the new compiler warnings? And it was sent to me
> despite that new warning?
>
> I realy want people to take a really hard look at functions that use
> arrays as arguments. It really is very misleading, even if it can look
> "prettier", and some people will argue that it's "documentation" about
> how the pointer is a particular size. But it's neither. It's basically
> just lying about what is going on, and the only thing it documents is
> "I don't know how to C". Misleading documentation isn't documentation,
> it's a mistake.
>
> I see it in that file for at least the functions rate_idx_match_mask()
> and rate_control_cap_mask(). I tried - and failed - to come up with a
> reasonable grep pattern to try to see how common it is, and I'm too
> lazy to add some sparse check for it.
>
> Please people. When I see these kinds of obviously bogus code
> problems, that just makes me very upset. Because it makes me worry
> about all the non-obvious stuff that I miss. Sadly, this time I had
> pushed out the merge early (because I wanted to test the wireless
> changes on my laptop), so now the bug is out there.
>
> I'm not sure what the practical *impact* of the bug is. Yes, it only
> traverses four or eight rate entries (depending on 32-bit or
> 64-bitness of the kernel) out of the ten that it sho
Re: [PATCH v2] iio: humidity: hts221: Fix sensor reads after resume
> AV_CONF register (RH & TEMP. oversampling ratio's) and CTRL1 register
> (ODR & BDU settings) values are lost after suspend.
>
> While the change in AV_CONF updates the sensor resolution modes
> (overriding the user configuration before the device went to suspend);
> loss of the contents of the CTRL1 register leads to failure in reading
> sensor output.
>
> This patch restores the AV_CONF & CTRL1 registers after resume.
>
Hi Shrirang,
I still suspect we have some hw issue (IoT Gateway) here (cc ST folks
to get more info) and anyway I guess the proposed patch will fix the issue
completely since OD and HL configuration is not restored properly.
Could you please double check HL/OD regs are properly preserved after resume
phase?
Regards,
Lorenzo
> Changes from v1:
> - Don't drop enabling the sensor during resume
> - Restore AV_CONF register along with CTRL1
> (As demonstrated with i2c register dumps of hts221 captured on Dell
> IoT Gateways 300x before suspend & after resume [1])
>
> Fixes: ffebe74b7c95 (iio: humidity: hts221: avoid useless ODR reconfiguration)
> Signed-off-by: Shrirang Bagul
>
> [v1] https://marc.info/?l=linux-iio&m=152506543000442&w=2
> [1] https://marc.info/?l=linux-iio&m=152534455701742&w=2
>
> This patch is based on:
> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git -b testing
> ---
> drivers/iio/humidity/hts221_core.c | 31 +-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/humidity/hts221_core.c
> b/drivers/iio/humidity/hts221_core.c
> index 166946d4978d..fc5599497f55 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -656,14 +656,43 @@ static int __maybe_unused hts221_resume(struct device
> *dev)
> {
> struct iio_dev *iio_dev = dev_get_drvdata(dev);
> struct hts221_hw *hw = iio_priv(iio_dev);
> + const struct hts221_avg *avg;
> + u8 data, idx;
> int err = 0;
>
> + /* Restore contents of AV_CONF (RH & TEMP. oversampling ratio's) */
> + avg = &hts221_avg_list[HTS221_SENSOR_H];
> + idx = hw->sensors[HTS221_SENSOR_H].cur_avg_idx;
> + data = avg->avg_avl[idx];
> + err = hts221_update_avg(hw, HTS221_SENSOR_H, data);
> + if (err < 0)
> + goto fail_err;
> +
> + avg = &hts221_avg_list[HTS221_SENSOR_T];
> + idx = hw->sensors[HTS221_SENSOR_T].cur_avg_idx;
> + data = avg->avg_avl[idx];
> + err = hts221_update_avg(hw, HTS221_SENSOR_T, data);
> + if (err < 0)
> + goto fail_err;
just return err, no need for goto statement
> +
> + /* Restore contents of CTRL1 (BDU & ODR) */
> + err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
> + HTS221_BDU_MASK,
> + FIELD_PREP(HTS221_BDU_MASK, 1));
> + if (err < 0)
> + goto fail_err;
> +
> + err = hts221_update_odr(hw, hw->odr);
> + if (err < 0)
> + goto fail_err;
> +
just return err, no need for goto statement
> if (hw->enabled)
> err = regmap_update_bits(hw->regmap, HTS221_REG_CNTRL1_ADDR,
>HTS221_ENABLE_MASK,
>FIELD_PREP(HTS221_ENABLE_MASK,
> true));
> - return err;
> +fail_err:
> + return err < 0 ? err : 0;
> }
>
> const struct dev_pm_ops hts221_pm_ops = {
> --
> 2.17.1
>
Re: [PATCH bpf-next v2] selftests/bpf: convert test_xdp_features.sh to test_progs
On Sep 13, Martin KaFai Lau wrote: > On 9/10/24 11:10 AM, Alexis Lothoré (eBPF Foundation) wrote: > > test_xdp_features.sh is a shell script allowing to test that xdp features > > advertised by an interface are indeed delivered. The test works by starting > > two instance of the same program, both attaching specific xdp programs to > > each side of a veth link, and then make those programs manage packets and > > collect stats to check whether tested XDP feature is indeed delivered or > > not. However this test is not integrated in test_progs framework and so can > > not run automatically in CI. > > > > Rewrite test_xdp_features to integrate it in test_progs so it can run > > automatically in CI. The main changes brought by the rewrite are the > > following: > > - instead of running to separated processes (each one managing either the > >tester veth or the DUT vet), run a single process > > - slightly change testing direction (v0 is the tester in local namespace, > >v1 is the Device Under Test in remote namespace) > > - group all tests previously managed by test_xdp_features as subtests (one > >per tested XDP feature). As a consequence, run only once some steps > >instead of once per subtest (eg: starting/stopping the udp server). On > >the contrary, make sure that each subtest properly cleans up its state > >(ie detach xdp programs, reset test stats, etc) > > - since there is now a single process, get rid of the "control" tcp channel > >used to configure DUT. Configuring the DUT now only consists in switching > >to DUT network namespace and run the relevant commands > > - since there is no more control channel, get rid of TLVs, keep only the > >CMD_ECHO packet type, and set it as a magic > > - simplify network setup: use only ipv6 instead of both ipv4 and ipv6, > >force static neighbours instead of waiting for autoconfiguration, do not > >force gro (fetch xdp features only once xdp programs are loaded instead) > > > > The existing XDP programs are reused, with some minor changes: > > - tester and dut stats maps are converted to global variables for easier > >usage > > - programs do not use TLV struct anymore but the magic replacing the echo > >command > > - avoid to accidentally make tests pass: drop packets instead of forwarding > >them to userspace when they do not match the expected payload > > - make sure to perform host <-> network endianness conversion on constants > >rather than packet parts > > > > Signed-off-by: Alexis Lothoré (eBPF Foundation) > > --- > > Changes in v2: > > - fix endianness management in userspace packet parsing (call htonl on > >constant rather than packet part) > > > > The xdp_features rewrite has been tested in a x86_64 qemu environment on my > > machine and in CI. In my environment, the test takes a bit less than 2s to > > execute. > > > ># ./test_progs -a xdp_features > >#561/1 xdp_features/XDP_PASS:OK > >#561/2 xdp_features/XDP_DROP:OK > >#561/3 xdp_features/XDP_ABORTED:OK > >#561/4 xdp_features/XDP_TX:OK > >#561/5 xdp_features/XDP_REDIRECT:OK > >#561/6 xdp_features/XDP_NDO_XMIT:OK > >#561 xdp_features:OK > >Summary: 1/6 PASSED, 0 SKIPPED, 0 FAILED > > --- > > tools/testing/selftests/bpf/.gitignore | 1 - > > tools/testing/selftests/bpf/Makefile | 10 +- > > .../selftests/bpf/prog_tests/xdp_features.c| 446 + > > tools/testing/selftests/bpf/progs/xdp_features.c | 49 +- > > tools/testing/selftests/bpf/test_xdp_features.sh | 107 --- > > tools/testing/selftests/bpf/xdp_features.c | 718 > > - Hi Alexis. sorry for the late reply. > > From the initial commit message of xdp_features.c, its primary usage is to > test a physical network device that supports a certain XDP features. > > iiuc, test_xdp_features.sh only uses the veth and veth will also be the only > device tested after moving to prog_tests/xdp_features.c? It is a reasonable > addition to test_progs for an end-to-end xdp test by using veth. However, > test_progs will not be able to test the physical network device. > > Lorenzo, is the xdp_features.c still used for device testing? > correct, xdp_features.c is intended to test the real xdp features supported by the NIC under test (DUT), not just the advertised ones (iirc that was a requisite to add xdp kernel feature support). For this reason we need two separated processes running on the tester device and on the DUT (they are usually two different devices). test_xdp_features.sh was just a simple test script used to develop xdp_features.c. What about extending xdp_features.c to integrate it in the CI? Regards, Lorenzo signature.asc Description: PGP signature
Re: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support
>
> On Mon, 15 Apr 2019 21:19:41 +0200
> Vitor Soares wrote:
>
> > For today the st_lsm6dsx driver support lsm6dso sensor only in
> > spi and i2c mode.
> >
> > The lsm6dso is also i3c capable so lets give i3c support to it.
> >
> > Signed-off-by: Vitor Soares
> > ---
> > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 67
> > +
> > 3 files changed, 75 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> >
> > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig
> > b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > index 094fd00..1ab9bbf 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > @@ -1,11 +1,12 @@
> >
> > config IIO_ST_LSM6DSX
> > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> > - depends on (I2C || SPI)
> > + depends on (I2C || SPI || I3C)
> > select IIO_BUFFER
> > select IIO_KFIFO_BUF
> > select IIO_ST_LSM6DSX_I2C if (I2C)
> > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> > + select IIO_ST_LSM6DSX_I3C if (I3C)
> > help
> > Say yes here to build support for STMicroelectronics LSM6DSx imu
> > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI
> > tristate
> > depends on IIO_ST_LSM6DSX
> > select REGMAP_SPI
> > +
> > +config IIO_ST_LSM6DSX_I3C
> > + tristate
> > + depends on IIO_ST_LSM6DSX
> > + select REGMAP_I3C
> > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile
> > b/drivers/iio/imu/st_lsm6dsx/Makefile
> > index e5f733c..c676965 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/Makefile
> > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
> > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > new file mode 100644
> > index 000..2df5e70
> > --- /dev/null
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > @@ -0,0 +1,67 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > + *
> > + * Author: Vitor Soares
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "st_lsm6dsx.h"
> > +
> > +#define NAME_SIZE32
> > +
> > +struct st_lsm6dsx_i3c_data {
> > + char name[NAME_SIZE];
>
> const char *name;
>
> > + enum st_lsm6dsx_hw_id id;
> > +};
> > +
> > +static const struct st_lsm6dsx_i3c_data hw_data[] = {
>
> No need to make it an array, and it should probably be named
> st_lsm6dso_data not hw_data.
I guess it will be used even for future devices so I would prefer to
make it an array with a 'general' name
Regards,
Lorenzo
>
> > + { ST_LSM6DSO_DEV_NAME, ST_LSM6DSO_ID },
> > +};
> > +
> > +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > +};
> > +
> > +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
> > +{
> > + const struct i3c_device_id *id = i3c_get_device_id(i3cdev);
>
> i3c_device_match_id(i3cdev,
>
> st_lsm6dsx_i3c_ids);
>
> > + const struct st_lsm6dsx_i3c_data *hw_data = id->data;
> > + struct regmap *regmap;
> > +
> > + regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config);
> > + if (IS_ERR(regmap)) {
> > + dev_err(&i3cdev->dev, "Failed to register i3c regmap %d\n",
> > + (int)PTR_ERR(regmap));
> > + return PTR_ERR(regmap);
> > + }
> > +
> > + return st_lsm6dsx_probe(&i3cdev->dev, 0, hw_data->id,
> > + hw_data->name, regmap);
> > +}
> > +
> > +
> > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > + I3C_DEVICE(0x0104, 0x006C, &hw_data[0]),
> > + { /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> > +
> > +static struct i3c_driver st_lsm6dsx_driver = {
> > + .driver.name = "st_lsm6dsx_i3c",
> > + .probe = st_lsm6dsx_i3c_probe,
> > + .id_table = st_lsm6dsx_i3c_ids,
>
> You should probably set the pm_ops here (st_lsm6dsx_pm_ops).
>
> > +};
> > +module_i3c_driver(st_lsm6dsx_driver);
> > +
> > +MODULE_AUTHOR("Vitor Soares ");
> > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > +MODULE_LICENSE("GPL v2");
> > +
>
> You can remove this blank line.
>
--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; ta
Re: mt76: mt7921: add MCU support
> Hi,
>
Hi Colin,
a fix for this issue has been already posted upstream:
https://patchwork.kernel.org/project/linux-wireless/patch/857ff74f736d4e593f5ad602cee7ac67ebfca5ca.1612867656.git.lore...@kernel.org/
Regards,
Lorenzo
> Static analysis with Coverity on linux-next has found an issue with the
> following commit:
>
> commit 1c099ab44727c8e42fe4de4d91b53cec3ef02860
> Author: Sean Wang
> Date: Thu Jan 28 03:33:39 2021 +0800
>
> mt76: mt7921: add MCU support
>
> The analysis is as follows:
>
> 390 static void
> 391 mt7921_mcu_tx_rate_report(struct mt7921_dev *dev, struct sk_buff *skb,
> 392 u16 wlan_idx)
> 393 {
> 394struct mt7921_mcu_wlan_info_event *wtbl_info =
> 395(struct mt7921_mcu_wlan_info_event *)(skb->data);
> 396struct rate_info rate = {};
> 397u8 curr_idx = wtbl_info->rate_info.rate_idx;
> 398u16 curr = le16_to_cpu(wtbl_info->rate_info.rate[curr_idx]);
> 399struct mt7921_mcu_peer_cap peer = wtbl_info->peer_cap;
> 400struct mt76_phy *mphy = &dev->mphy;
>
>1. var_decl: Declaring variable stats without initializer.
>
> 401struct mt7921_sta_stats *stats;
> 402struct mt7921_sta *msta;
> 403struct mt76_wcid *wcid;
> 404
>
>2. Condition wlan_idx >= 288, taking false branch.
>
> 405if (wlan_idx >= MT76_N_WCIDS)
> 406return;
>
>3. Condition 0 /* !sizeof ((*dev).mt76.wcid[wlan_idx]) == sizeof
> (char) || sizeof ((*dev).mt76.wcid[wlan_idx]) == sizeof (short)) ||
> sizeof ((*dev).mt76.wcid[wlan_idx]) == sizeof (int)) || sizeof
> ((*dev).mt76.wcid[wlan_idx]) == sizeof (long)) || sizeof
> ((*dev).mt76.wcid[wlan_idx]) == sizeof (long long)) */, taking false branch.
>
>4. Condition debug_lockdep_rcu_enabled(), taking true branch.
>5. Condition !__warned, taking true branch.
>6. Condition 0, taking false branch.
>7. Condition rcu_read_lock_held(), taking false branch.
> 407wcid = rcu_dereference(dev->mt76.wcid[wlan_idx]);
>8. Condition !wcid, taking true branch.
> 408if (!wcid) {
>
> Uninitialized pointer write (UNINIT)
>9. uninit_use: Using uninitialized value stats.
>
> 409stats->tx_rate = rate;
> 410return;
> 411}
>
> Line 409 dereferences pointer stats, however, this pointer has not yet
> been initialized. The initialization occurs later:
>
> 413msta = container_of(wcid, struct mt7921_sta, wcid);
> 414stats = &msta->stats;
>
> Colin
signature.asc
Description: PGP signature
Re: [PATCH] mt76: Fix queue ID variable types after mcu queue split
> Clang warns in both mt7615 and mt7915:
>
> drivers/net/wireless/mediatek/mt76/mt7915/mcu.c:271:9: warning: implicit
> conversion from enumeration type 'enum mt76_mcuq_id' to different
> enumeration type 'enum mt76_txq_id' [-Wenum-conversion]
> txq = MT_MCUQ_FWDL;
> ~ ^~~~
> drivers/net/wireless/mediatek/mt76/mt7915/mcu.c:278:9: warning: implicit
> conversion from enumeration type 'enum mt76_mcuq_id' to different
> enumeration type 'enum mt76_txq_id' [-Wenum-conversion]
> txq = MT_MCUQ_WA;
> ~ ^~
> drivers/net/wireless/mediatek/mt76/mt7915/mcu.c:282:9: warning: implicit
> conversion from enumeration type 'enum mt76_mcuq_id' to different
> enumeration type 'enum mt76_txq_id' [-Wenum-conversion]
> txq = MT_MCUQ_WM;
> ~ ^~
> 3 warnings generated.
>
> drivers/net/wireless/mediatek/mt76/mt7615/mcu.c:238:9: warning: implicit
> conversion from enumeration type 'enum mt76_mcuq_id' to different
> enumeration type 'enum mt76_txq_id' [-Wenum-conversion]
> qid = MT_MCUQ_WM;
> ~ ^~
> drivers/net/wireless/mediatek/mt76/mt7615/mcu.c:240:9: warning: implicit
> conversion from enumeration type 'enum mt76_mcuq_id' to different
> enumeration type 'enum mt76_txq_id' [-Wenum-conversion]
> qid = MT_MCUQ_FWDL;
> ~ ^~~~
> 2 warnings generated.
>
> Use the proper type for the queue ID variables to fix these warnings.
> Additionally, rename the txq variable in mt7915_mcu_send_message to be
> more neutral like mt7615_mcu_send_message.
>
> Fixes: e637763b606b ("mt76: move mcu queues to mt76_dev q_mcu array")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1229
> Signed-off-by: Nathan Chancellor
Acked-by: Lorenzo Bianconi
> ---
> drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 2 +-
> drivers/net/wireless/mediatek/mt76/mt7915/mcu.c | 10 +-
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> index a44b7766dec6..c13547841a4e 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> @@ -231,7 +231,7 @@ mt7615_mcu_send_message(struct mt76_dev *mdev, struct
> sk_buff *skb,
> int cmd, int *seq)
> {
> struct mt7615_dev *dev = container_of(mdev, struct mt7615_dev, mt76);
> - enum mt76_txq_id qid;
> + enum mt76_mcuq_id qid;
>
> mt7615_mcu_fill_msg(dev, skb, cmd, seq);
> if (test_bit(MT76_STATE_MCU_RUNNING, &dev->mphy.state))
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> index 5fdd1a6d32ee..e211a2bd4d3c 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> @@ -256,7 +256,7 @@ mt7915_mcu_send_message(struct mt76_dev *mdev, struct
> sk_buff *skb,
> struct mt7915_dev *dev = container_of(mdev, struct mt7915_dev, mt76);
> struct mt7915_mcu_txd *mcu_txd;
> u8 seq, pkt_fmt, qidx;
> - enum mt76_txq_id txq;
> + enum mt76_mcuq_id qid;
> __le32 *txd;
> u32 val;
>
> @@ -268,18 +268,18 @@ mt7915_mcu_send_message(struct mt76_dev *mdev, struct
> sk_buff *skb,
> seq = ++dev->mt76.mcu.msg_seq & 0xf;
>
> if (cmd == -MCU_CMD_FW_SCATTER) {
> - txq = MT_MCUQ_FWDL;
> + qid = MT_MCUQ_FWDL;
> goto exit;
> }
>
> mcu_txd = (struct mt7915_mcu_txd *)skb_push(skb, sizeof(*mcu_txd));
>
> if (test_bit(MT76_STATE_MCU_RUNNING, &dev->mphy.state)) {
> - txq = MT_MCUQ_WA;
> + qid = MT_MCUQ_WA;
> qidx = MT_TX_MCU_PORT_RX_Q0;
> pkt_fmt = MT_TX_TYPE_CMD;
> } else {
> - txq = MT_MCUQ_WM;
> + qid = MT_MCUQ_WM;
> qidx = MT_TX_MCU_PORT_RX_Q0;
> pkt_fmt = MT_TX_TYPE_CMD;
> }
> @@ -326,7 +326,7 @@ mt7915_mcu_send_message(struct mt76_dev *mdev, struct
> sk_buff *skb,
> if (wait_seq)
> *wait_seq = seq;
>
> - return mt76_tx_queue_skb_raw(dev, mdev->q_mcu[txq], skb, 0);
> + return mt76_tx_queue_skb_raw(dev, mdev->q_mcu[qid], skb, 0);
> }
>
> static void
>
> base-commit: 5c8fe583cce542aa0b84adc939ce85293de36e5e
> --
> 2.30.0
>
signature.asc
Description: PGP signature
Re: [PATCH net-next 6/6] mvneta: recycle buffers
[...] > > diff --git a/drivers/net/ethernet/marvell/mvneta.c > > b/drivers/net/ethernet/marvell/mvneta.c > > index a635cf84608a..8b3250394703 100644 > > --- a/drivers/net/ethernet/marvell/mvneta.c > > +++ b/drivers/net/ethernet/marvell/mvneta.c > > @@ -2332,7 +2332,7 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct > > mvneta_rx_queue *rxq, > > if (!skb) > > return ERR_PTR(-ENOMEM); > > > > - page_pool_release_page(rxq->page_pool, virt_to_page(xdp->data)); > > + skb_mark_for_recycle(skb, virt_to_page(xdp->data), &xdp->rxq->mem); > > > > skb_reserve(skb, xdp->data - xdp->data_hard_start); > > skb_put(skb, xdp->data_end - xdp->data); > > @@ -2344,7 +2344,7 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct > > mvneta_rx_queue *rxq, > > skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, > > skb_frag_page(frag), skb_frag_off(frag), > > skb_frag_size(frag), PAGE_SIZE); > > - page_pool_release_page(rxq->page_pool, skb_frag_page(frag)); > > + skb_mark_for_recycle(skb, skb_frag_page(frag), &xdp->rxq->mem); > > } > > > > return skb; > > This cause skb_mark_for_recycle() to set 'skb->pp_recycle=1' multiple > times, for the same SKB. (copy-pasted function below signature to help > reviewers). > > This makes me question if we need an API for setting this per page > fragment? > Or if the API skb_mark_for_recycle() need to walk the page fragments in > the SKB and set the info stored in the page for each? Considering just performances, I guess it is better open-code here since the driver already performs a loop over fragments to build the skb, but I guess this approach is quite risky and I would prefer to have a single utility routine to take care of linear area + fragments. What do you think? Regards, Lorenzo > > > -- > Best regards, > Jesper Dangaard Brouer > MSc.CS, Principal Kernel Engineer at Red Hat > LinkedIn: http://www.linkedin.com/in/brouer > signature.asc Description: PGP signature
Re: [PATCH] iio: imu: st_lsm6dsx: Fix FIFO diff mask for tagged fifo
> On Thu, 22 Aug 2019 15:22:19 +0200
> Mario Tesi wrote:
>
> > From: mario tesi
> >
> > According to the latest version of datasheet the mask
> > for number of unread sensor data in FIFO_STATUS registers
> > has been extended to 10 bits
> >
> > The devices involved are:
> > - LSM6DSO
> > - LSM6DSOX
> > - ASM330LHH
> > - LSM6DSR
> > - ISM330DHCX
> >
> > Signed-off-by: mario tesi
>
> Seems straight forward and should be side effect free I think.
> Hence I won't wait for Lorenzo to take a look (though there
> is still a small window for comments whilst the autobuilders
> poke at it!)
>
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to take a look.
>
> Thanks,
Acked-by: Lorenzo Bianconi
>
> Jonathan
>
> > ---
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > index 85824d6..47b77d0 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > @@ -497,7 +497,7 @@ static const struct st_lsm6dsx_settings
> > st_lsm6dsx_sensor_settings[] = {
> > },
> > .fifo_diff = {
> > .addr = 0x3a,
> > - .mask = GENMASK(8, 0),
> > + .mask = GENMASK(9, 0),
> > },
> > .th_wl = 1,
> > },
> > @@ -623,7 +623,7 @@ static const struct st_lsm6dsx_settings
> > st_lsm6dsx_sensor_settings[] = {
> > },
> > .fifo_diff = {
> > .addr = 0x3a,
> > - .mask = GENMASK(8, 0),
> > + .mask = GENMASK(9, 0),
> > },
> > .th_wl = 1,
> > },
> > @@ -726,7 +726,7 @@ static const struct st_lsm6dsx_settings
> > st_lsm6dsx_sensor_settings[] = {
> > },
> > .fifo_diff = {
> > .addr = 0x3a,
> > - .mask = GENMASK(8, 0),
> > + .mask = GENMASK(9, 0),
> > },
> > .th_wl = 1,
> > },
>
signature.asc
Description: PGP signature
Re: [PATCH] mt7601u: phy: simplify zero check on val
> From: Colin Ian King
>
> Currently the zero check on val to break out of a loop
> is a little obscure. Replace the val is zero and break check
> with a loop while value is non-zero.
>
> Signed-off-by: Colin Ian King
> ---
> drivers/net/wireless/mediatek/mt7601u/phy.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt7601u/phy.c
> b/drivers/net/wireless/mediatek/mt7601u/phy.c
> index 06f5702ab4bd..4e0e473caae1 100644
> --- a/drivers/net/wireless/mediatek/mt7601u/phy.c
> +++ b/drivers/net/wireless/mediatek/mt7601u/phy.c
> @@ -213,9 +213,7 @@ int mt7601u_wait_bbp_ready(struct mt7601u_dev *dev)
>
> do {
> val = mt7601u_bbp_rr(dev, MT_BBP_REG_VERSION);
> - if (val && ~val)
> - break;
I think this is not correct since (not considering the cast) we should break
from the loop if val != 0 and val != 0xff, so the right approach I guess is:
diff --git a/drivers/net/wireless/mediatek/mt7601u/phy.c
b/drivers/net/wireless/mediatek/mt7601u/phy.c
index 06f5702ab4bd..d863ab4a66c9 100644
--- a/drivers/net/wireless/mediatek/mt7601u/phy.c
+++ b/drivers/net/wireless/mediatek/mt7601u/phy.c
@@ -213,7 +213,7 @@ int mt7601u_wait_bbp_ready(struct mt7601u_dev *dev)
do {
val = mt7601u_bbp_rr(dev, MT_BBP_REG_VERSION);
- if (val && ~val)
+ if (val && val != 0xff)
break;
} while (--i);
> - } while (--i);
> + } while (val && --i);
>
> if (!i) {
> dev_err(dev->dev, "Error: BBP is not ready\n");
> --
> 2.20.1
>
signature.asc
Description: PGP signature
Re: [PATCH v4 1/3] iio: imu: st_lsm6sdx: move register definitions to sensor_settings struct
> Move some register definitions to the per-device array of struct
> st_lsm6dsx_sensor_settings in order to simplify adding new sensor
> devices to the driver.
>
> Also, remove completely unused register definitions.
>
> Signed-off-by: Martin Kepplinger
Acked-by: Lorenzo Bianconi
> ---
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 6
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 31 ++--
> 2 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index 4e8e67ae1632..c8f333902eb7 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -200,6 +200,9 @@ struct st_lsm6dsx_ext_dev_settings {
> /**
> * struct st_lsm6dsx_settings - ST IMU sensor settings
> * @wai: Sensor WhoAmI default value.
> + * @int1_addr: Control Register address for INT1
> + * @int2_addr: Control Register address for INT2
> + * @reset_addr: register address for reset/reboot
> * @max_fifo_size: Sensor max fifo length in FIFO words.
> * @id: List of hw id/device name supported by the driver configuration.
> * @channels: IIO channels supported by the device.
> @@ -213,6 +216,9 @@ struct st_lsm6dsx_ext_dev_settings {
> */
> struct st_lsm6dsx_settings {
> u8 wai;
> + u8 int1_addr;
> + u8 int2_addr;
> + u8 reset_addr;
> u16 max_fifo_size;
> struct {
> enum st_lsm6dsx_hw_id hw_id;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 85824d6739ee..56e1c5262a2c 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -49,17 +49,12 @@
>
> #include "st_lsm6dsx.h"
>
> -#define ST_LSM6DSX_REG_INT1_ADDR 0x0d
> -#define ST_LSM6DSX_REG_INT2_ADDR 0x0e
> #define ST_LSM6DSX_REG_FIFO_FTH_IRQ_MASK BIT(3)
> #define ST_LSM6DSX_REG_WHOAMI_ADDR 0x0f
> -#define ST_LSM6DSX_REG_RESET_ADDR0x12
> #define ST_LSM6DSX_REG_RESET_MASKBIT(0)
> #define ST_LSM6DSX_REG_BOOT_MASK BIT(7)
> #define ST_LSM6DSX_REG_BDU_ADDR 0x12
> #define ST_LSM6DSX_REG_BDU_MASK BIT(6)
> -#define ST_LSM6DSX_REG_INT2_ON_INT1_ADDR 0x13
> -#define ST_LSM6DSX_REG_INT2_ON_INT1_MASK BIT(5)
>
> static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
> ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
> @@ -78,6 +73,9 @@ static const struct iio_chan_spec
> st_lsm6dsx_gyro_channels[] = {
> static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
> {
> .wai = 0x69,
> + .int1_addr = 0x0d,
> + .int2_addr = 0x0e,
> + .reset_addr = 0x12,
> .max_fifo_size = 1365,
> .id = {
> {
> @@ -186,6 +184,9 @@ static const struct st_lsm6dsx_settings
> st_lsm6dsx_sensor_settings[] = {
> },
> {
> .wai = 0x69,
> + .int1_addr = 0x0d,
> + .int2_addr = 0x0e,
> + .reset_addr = 0x12,
> .max_fifo_size = 682,
> .id = {
> {
> @@ -294,6 +295,9 @@ static const struct st_lsm6dsx_settings
> st_lsm6dsx_sensor_settings[] = {
> },
> {
> .wai = 0x6a,
> + .int1_addr = 0x0d,
> + .int2_addr = 0x0e,
> + .reset_addr = 0x12,
> .max_fifo_size = 682,
> .id = {
> {
> @@ -411,6 +415,9 @@ static const struct st_lsm6dsx_settings
> st_lsm6dsx_sensor_settings[] = {
> },
> {
> .wai = 0x6c,
> + .int1_addr = 0x0d,
> + .int2_addr = 0x0e,
> + .reset_addr = 0x12,
> .max_fifo_size = 512,
> .id = {
> {
> @@ -540,6 +547,9 @@ static const struct st_lsm6dsx_settings
> st_lsm6dsx_sensor_settings[] = {
> },
> {
> .wai = 0x6b,
> + .int1_addr = 0x0d,
> + .int2_addr = 0x0e,
> + .reset_addr = 0x12,
> .max_fifo_size = 512,
> .id = {
> {
> @@ -640,6 +650,9 @@ static const struct st_lsm6dsx_settings
> st_lsm6dsx_sensor_settings[] = {
> },
> {
> .wai = 0x6b,
> + .int1_addr = 0x0d,
> + .int2_addr = 0x0e,
> + .reset_addr = 0x12,
> .max_fifo_siz
Re: [PATCH 2/3] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1
> The LSM9DS1's accelerometer / gyroscope unit and it's magnetometer (separately
> supported in iio/magnetometer/st_magn*) are located on a separate i2c
> addresses
> on the bus.
>
> For the datasheet, see https://www.st.com/resource/en/datasheet/lsm9ds1.pdf
>
> Treat it just like the LSM6* devices and, despite it's name, hook it up
> to the st_lsm6dsx driver, using it's basic functionality.
>
> Signed-off-by: Martin Kepplinger
> ---
>
> What do you think about an addition like this? How confusing is it to support
> an LSM9 module by the lsm6 driver, despite it's name? It requires almost no
> code, so why not think about it, right?
I am fine with (it was on my ToDo list, so thanks for working on this).
I have just posted the following series that will help you adding support for
LSM9DS1
https://patchwork.kernel.org/cover/11045061/
I think you just need to take care of gyro channels allocating iio devices (you
probably need to pass hw_id to st_lsm6dsx_alloc_iiodev())
>
> Oh, I'm not 100% convinced by my new "if" in probe(), but even that is
> not too confusing I guess.
>
> thanks,
>
>martin
>
> p.s.: todos:
> * hook up the fifo / buffer / trigger functionality,
> * (off-topic a bit) move the (currently strange) gyro-only support
> for lsm9ds0 to this driver as well.
Regarding FIFO I guess it is enough to set decimator factor to 1 for both accel
and gyro.
Regards,
Lorenzo
>
>
>
> drivers/iio/imu/st_lsm6dsx/Kconfig | 3 +-
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 4 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 105 ++-
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c | 5 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c | 5 +
> 5 files changed, 117 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig
> b/drivers/iio/imu/st_lsm6dsx/Kconfig
> index 002a423eae52..0b5a568e4c16 100644
> --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> @@ -10,7 +10,8 @@ config IIO_ST_LSM6DSX
> help
> Say yes here to build support for STMicroelectronics LSM6DSx imu
> sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> - ism330dlc, lsm6dso, lsm6dsox, asm330lhh, lsm6dsr
> + ism330dlc, lsm6dso, lsm6dsox, asm330lhh, lsm6dsr and the
> + accelerometer/gyroscope of lsm9ds1.
>
> To compile this driver as a module, choose M here: the module
> will be called st_lsm6dsx.
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index f072ac14f213..8af9641260fa 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -22,6 +22,7 @@
> #define ST_ASM330LHH_DEV_NAME"asm330lhh"
> #define ST_LSM6DSOX_DEV_NAME "lsm6dsox"
> #define ST_LSM6DSR_DEV_NAME "lsm6dsr"
> +#define ST_LSM9DS1_DEV_NAME "lsm9ds1"
>
> enum st_lsm6dsx_hw_id {
> ST_LSM6DS3_ID,
> @@ -33,6 +34,7 @@ enum st_lsm6dsx_hw_id {
> ST_ASM330LHH_ID,
> ST_LSM6DSOX_ID,
> ST_LSM6DSR_ID,
> + ST_LSM9DS1_ID,
> ST_LSM6DSX_MAX_ID,
> };
>
> @@ -230,6 +232,8 @@ enum st_lsm6dsx_sensor_id {
> ST_LSM6DSX_ID_EXT0,
> ST_LSM6DSX_ID_EXT1,
> ST_LSM6DSX_ID_EXT2,
> + ST_LSM9DSX_ID_GYRO,
> + ST_LSM9DSX_ID_ACC,
> ST_LSM6DSX_ID_MAX,
> };
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 7a4fe70a8f20..6acfe63073de 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -10,6 +10,8 @@
> * +-125/+-245/+-500/+-1000/+-2000 dps
> * LSM6DSx series has an integrated First-In-First-Out (FIFO) buffer
> * allowing dynamic batching of sensor data.
> + * LSM9DSx series is similar but includes an additional magnetometer, handled
> + * by a different driver.
> *
> * Supported sensors:
> * - LSM6DS3:
> @@ -30,6 +32,13 @@
> * - Gyroscope supported full-scale [dps]: +-125/+-245/+-500/+-1000/+-2000
> * - FIFO size: 3KB
> *
> + * - LSM9DS1:
> + * - Accelerometer supported ODR [Hz]: 10, 50, 119, 238, 476, 952
> + * - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16
> + * - Gyroscope supported ODR [Hz]: 15, 60, 119, 238, 476, 952
> + * - Gyroscope supported full-scale [dps]: +-245/+-500/+-2000
> + * - FIFO size: 32
> + *
> * Copyright 2016 STMicroelectronics Inc.
> *
> * Lorenzo Bianconi
> @@ -64,6 +73,10 @@
> #define ST_LSM6DSX_REG_GYR
Re: [PATCHv2 2/3] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1
> The LSM9DS1's accelerometer / gyroscope unit and it's magnetometer (separately
> supported in iio/magnetometer/st_magn*) are located on a separate i2c
> addresses
> on the bus.
>
> For the datasheet, see https://www.st.com/resource/en/datasheet/lsm9ds1.pdf
>
> Treat it just like the LSM6* devices and, despite it's name, hook it up
> to the st_lsm6dsx driver, using it's basic functionality.
I think LSM9DS1 relies on LSM6DS0 for acc and gyro part so I guess we can use
this name here, what do you think?
>
> Signed-off-by: Martin Kepplinger
> ---
>
>
> This is already based on Lorenzo's recent changes:
> https://lore.kernel.org/linux-iio/853f216a-7814-cb79-180b-078ac5e8a...@puri.sm/T/#u
> https://lore.kernel.org/linux-iio/501b0db9-63cb-905c-c09b-682eb73f1...@puri.sm/T/#u
>
> revision history:
> v2: overall further simplification
>
> thanks
>martin
>
>
>
> drivers/iio/imu/st_lsm6dsx/Kconfig | 1 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 94 +++-
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c | 5 ++
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c | 5 ++
> 5 files changed, 104 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig
> b/drivers/iio/imu/st_lsm6dsx/Kconfig
> index 2d8b2e1edfce..4a57bfb3c12e 100644
> --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> @@ -11,6 +11,7 @@ config IIO_ST_LSM6DSX
> Say yes here to build support for STMicroelectronics LSM6DSx imu
> sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> ism330dlc, lsm6dso, lsm6dsox, asm330lhh, lsm6dsr, lsm6ds3tr-c
> + and the accelerometer/gyroscope of lsm9ds1.
>
> To compile this driver as a module, choose M here: the module
> will be called st_lsm6dsx.
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index 3c47f5d27d30..9a30cc717de2 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -23,6 +23,7 @@
> #define ST_LSM6DSOX_DEV_NAME "lsm6dsox"
> #define ST_LSM6DSR_DEV_NAME "lsm6dsr"
> #define ST_LSM6DS3TRC_DEV_NAME "lsm6ds3tr-c"
> +#define ST_LSM9DS1_DEV_NAME "lsm9ds1"
>
> enum st_lsm6dsx_hw_id {
> ST_LSM6DS3_ID,
> @@ -35,6 +36,7 @@ enum st_lsm6dsx_hw_id {
> ST_LSM6DSOX_ID,
> ST_LSM6DSR_ID,
> ST_LSM6DS3TRC_ID,
> + ST_LSM9DS1_ID,
> ST_LSM6DSX_MAX_ID,
> };
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index e0d2149625cc..2f3d2bf25646 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -10,6 +10,8 @@
> * +-125/+-245/+-500/+-1000/+-2000 dps
> * LSM6DSx series has an integrated First-In-First-Out (FIFO) buffer
> * allowing dynamic batching of sensor data.
> + * LSM9DSx series is similar but includes an additional magnetometer, handled
> + * by a different driver.
> *
> * Supported sensors:
> * - LSM6DS3:
> @@ -30,6 +32,13 @@
> * - Gyroscope supported full-scale [dps]: +-125/+-245/+-500/+-1000/+-2000
> * - FIFO size: 3KB
> *
> + * - LSM9DS1:
> + * - Accelerometer supported ODR [Hz]: 10, 50, 119, 238, 476, 952
> + * - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16
> + * - Gyroscope supported ODR [Hz]: 15, 60, 119, 238, 476, 952
> + * - Gyroscope supported full-scale [dps]: +-245/+-500/+-2000
> + * - FIFO size: 32
> + *
> * Copyright 2016 STMicroelectronics Inc.
> *
> * Lorenzo Bianconi
> @@ -64,7 +73,72 @@
> #define ST_LSM6DSX_REG_GYRO_OUT_Y_L_ADDR 0x24
> #define ST_LSM6DSX_REG_GYRO_OUT_Z_L_ADDR 0x26
>
> +#define ST_LSM9DSX_REG_GYRO_OUT_X_L_ADDR 0x18
> +#define ST_LSM9DSX_REG_GYRO_OUT_Y_L_ADDR 0x1a
> +#define ST_LSM9DSX_REG_GYRO_OUT_Z_L_ADDR 0x1c
> +
> static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
> + {
> + .wai = 0x68,
> + .int1_addr = 0x0c,
> + .int2_addr = 0x0d,
> + .reset_addr = 0x22,
> + .max_fifo_size = 32,
> + .id = {
> + {
> + .hw_id = ST_LSM9DS1_ID,
> + .name = ST_LSM9DS1_DEV_NAME,
> + },
> + },
> + .odr_table = {
> + [ST_LSM6DSX_ID_ACC] = {
> +
Re: mt76x2e hardware restart
> > Hello.
> >
> > On 15.10.2019 18:52, Oleksandr Natalenko wrote:
> > > Thanks for the answer and the IRC discussion. As agreed I've applied
> > > [1] and [2], and have just swapped the card to try it again. So far,
> > > it works fine in 5 GHz band in 802.11ac mode as an AP.
> > >
> > > I'll give it more load with my phone over evening, and we can discuss
> > > what to do next (if needed) tomorrow again. Or feel free to drop me an
> > > email today.
> > >
> > > Thanks for your efforts.
> > >
> > > [1]
> > > https://github.com/LorenzoBianconi/wireless-drivers-next/commit/cf3436c42a297967235a9c9778620c585100529e.patch
> > > [2]
> > > https://github.com/LorenzoBianconi/wireless-drivers-next/commit/aad256eb62620f9646d39c1aa69234f50c89eed8.patch
> >
> > As agreed, here are iperf3 results, AP to STA distance is 2 meters.
> >
> > Client sends, TCP:
> >
> > [ ID] Interval Transfer Bitrate Retr
> > [ 5] 0.00-10.00 sec 70.4 MBytes 59.0 Mbits/sec 3800
> > sender
> > [ 5] 0.00-10.03 sec 70.0 MBytes 58.6 Mbits/sec
> > receiver
> >
> > Client receives, TCP:
> >
> > [ ID] Interval Transfer Bitrate Retr
> > [ 5] 0.00-10.06 sec 196 MBytes 163 Mbits/sec 3081
> > sender
> > [ 5] 0.00-10.01 sec 191 MBytes 160 Mbits/sec
> > receiver
> >
> > Client sends, UDP, 128 streams:
> >
> > [ ID] Interval Transfer Bitrate JitterLost/Total
> > Datagrams
> > [SUM] 0.00-10.00 sec 160 MBytes 134 Mbits/sec 0.000 ms 0/115894
> > (0%) sender
> > [SUM] 0.00-10.01 sec 160 MBytes 134 Mbits/sec 0.347 ms 0/115892
> > (0%) receiver
> >
> > Client receives, UDP, 128 streams:
> >
> > [ ID] Interval Transfer Bitrate JitterLost/Total
> > Datagrams
> > [SUM] 0.00-10.01 sec 119 MBytes 99.4 Mbits/sec 0.000 ms 0/85888 (0%)
> > sender
> > [SUM] 0.00-10.00 sec 119 MBytes 99.5 Mbits/sec 0.877 ms 0/85888 (0%)
> > receiver
> >
> > Given the HW is not the most powerful, the key point here is that nothing
> > crashed after doing these tests.
>
> Hi Oleksandr,
>
> thx a lot for testing these 2 patches. Now we need to understand why the chip
> hangs if we enable scatter gather dma transfer on x86 while it is working fine
> on multiple mips/arm devices (patch 2/2 just disable it for debugging).
Hi Oleksandr,
I think I spotted the SG issue on mt76x2e. Could you please:
- keep pcie_aspm patch I sent
- remove the debug patch where I disabled TX Scatter-Gather on mt76x2e
- apply the following patch
Regards,
Lorenzo
mt76: dma: fix buffer unmap with non-linear skbs
mt76 dma layer is supposed to unmap skb data buffers while keep txwi mapped
on hw dma ring. At the moment mt76 wrongly unmap txwi or does not unmap data
fragments in even positions for non-linear skbs. This issue may result in hw
hangs with A-MSUD if the system relies on IOMMU or SWIOTLB.
Fix this behaviour marking first and last queue entries introducing
MT_QUEUE_ENTRY_FIRST and MT_QUEUE_ENTRY_LAST flags and properly unmap
data fragments
Fixes: 17f1de56df05 ("mt76: add common code shared between multiple chipsets")
Signed-off-by: Lorenzo Bianconi
---
drivers/net/wireless/mediatek/mt76/dma.c | 33 +--
drivers/net/wireless/mediatek/mt76/mt76.h | 3 +++
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/dma.c
b/drivers/net/wireless/mediatek/mt76/dma.c
index 4da7cffbab29..a3026a0ca8c5 100644
--- a/drivers/net/wireless/mediatek/mt76/dma.c
+++ b/drivers/net/wireless/mediatek/mt76/dma.c
@@ -54,7 +54,7 @@ mt76_dma_add_buf(struct mt76_dev *dev, struct mt76_queue *q,
int i, idx = -1;
if (txwi)
- q->entry[q->head].txwi = DMA_DUMMY_DATA;
+ q->entry[q->head].flags = MT_QUEUE_ENTRY_FIRST;
for (i = 0; i < nbufs; i += 2, buf += 2) {
u32 buf0 = buf[0].addr, buf1 = 0;
@@ -83,6 +83,7 @@ mt76_dma_add_buf(struct mt76_dev *dev, struct mt76_queue *q,
q->queued++;
}
+ q->entry[idx].flags |= MT_QUEUE_ENTRY_LAST;
q->entry[idx].txwi = txwi;
q->entry[idx].skb = skb;
@@ -93,27 +94,31 @@ static void
mt76_dma_tx_cleanup_idx(struct mt76_dev *dev, struct mt76_queue *q, int idx,
struct mt76_queue_entry *prev_e)
{
+ __le32 addr, __ctrl = READ_ONCE(q->desc[idx].ctrl);
struct mt76_queue_entry *e = &q->entry[idx];
- __le32 __ctrl = READ_ONCE(q->desc[idx].ctrl);
- u32 ctr
Re: mt76x2e hardware restart
> On 19.09.2019 23:22, Oleksandr Natalenko wrote:
> > It checks for TX hang here:
> >
> > === mt76x02_mmio.c
> > 557 void mt76x02_wdt_work(struct work_struct *work)
> > 558 {
> > ...
> > 562 mt76x02_check_tx_hang(dev);
> > ===
>
> I've commented out the watchdog here ^^, and the card is not resetted any
> more, but similarly it stops working shortly after the first client
> connects. So, indeed, it must be some hang in the HW, and wdt seems to do a
> correct job.
>
> Is it even debuggable/fixable from the driver?
Hi Oleksandr,
sorry for the delay. Felix and me worked on this issue today. Could you please
try if the following patch fixes your issue?
Regards,
Lorenzo
From cf3436c42a297967235a9c9778620c585100529e Mon Sep 17 00:00:00 2001
Message-Id:
From: Lorenzo Bianconi
Date: Sat, 12 Oct 2019 17:32:57 +0200
Subject: [PATCH] mt76: mt76x2: disable pcie_aspm by default
On same device (e.g. U7612E-H1) PCIE_ASPM causes continues mcu hangs and
instability. This patch disable PCIE_ASPM by default. This patch has
been successfully tested on U7612E-H1 mini-pice card
Signed-off-by: Felix Fietkau
Signed-off-by: Lorenzo Bianconi
---
drivers/net/wireless/mediatek/mt76/mmio.c | 48 +++
drivers/net/wireless/mediatek/mt76/mt76.h | 1 +
.../net/wireless/mediatek/mt76/mt76x2/pci.c | 2 +
3 files changed, 51 insertions(+)
diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c
b/drivers/net/wireless/mediatek/mt76/mmio.c
index 1c974df1fe25..8e1dbc1903f3 100644
--- a/drivers/net/wireless/mediatek/mt76/mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mmio.c
@@ -3,6 +3,9 @@
* Copyright (C) 2016 Felix Fietkau
*/
+#include
+#include
+
#include "mt76.h"
#include "trace.h"
@@ -78,6 +81,51 @@ void mt76_set_irq_mask(struct mt76_dev *dev, u32 addr,
}
EXPORT_SYMBOL_GPL(mt76_set_irq_mask);
+void mt76_mmio_disable_aspm(struct pci_dev *pdev)
+{
+ struct pci_dev *parent = pdev->bus->self;
+ u16 aspm_conf, parent_aspm_conf = 0;
+
+ pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &aspm_conf);
+ aspm_conf &= PCI_EXP_LNKCTL_ASPMC;
+ if (parent) {
+ pcie_capability_read_word(parent, PCI_EXP_LNKCTL,
+ &parent_aspm_conf);
+ parent_aspm_conf &= PCI_EXP_LNKCTL_ASPMC;
+ }
+
+ if (!aspm_conf && (!parent || !parent_aspm_conf)) {
+ /* aspm already disabled */
+ return;
+ }
+
+ dev_info(&pdev->dev, "disabling ASPM %s %s\n",
+(aspm_conf & PCI_EXP_LNKCTL_ASPM_L0S) ? "L0s" : "",
+(aspm_conf & PCI_EXP_LNKCTL_ASPM_L1) ? "L1" : "");
+
+#ifdef CONFIG_PCIEASPM
+ pci_disable_link_state(pdev, aspm_conf);
+
+ /* Double-check ASPM control. If not disabled by the above, the
+* BIOS is preventing that from happening (or CONFIG_PCIEASPM is
+* not enabled); override by writing PCI config space directly.
+*/
+ pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &aspm_conf);
+ if (!(aspm_conf & PCI_EXP_LNKCTL_ASPMC))
+ return;
+#endif /* CONFIG_PCIEASPM */
+
+ /* Both device and parent should have the same ASPM setting.
+* Disable ASPM in downstream component first and then upstream.
+*/
+ pcie_capability_clear_word(pdev, PCI_EXP_LNKCTL, aspm_conf);
+
+ if (parent)
+ pcie_capability_clear_word(parent, PCI_EXP_LNKCTL,
+ aspm_conf);
+}
+EXPORT_SYMBOL_GPL(mt76_mmio_disable_aspm);
+
void mt76_mmio_init(struct mt76_dev *dev, void __iomem *regs)
{
static const struct mt76_bus_ops mt76_mmio_ops = {
diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h
b/drivers/net/wireless/mediatek/mt76/mt76.h
index 8bcc7f21e83c..e95a5893f93b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -596,6 +596,7 @@ bool __mt76_poll_msec(struct mt76_dev *dev, u32 offset, u32
mask, u32 val,
#define mt76_poll_msec(dev, ...) __mt76_poll_msec(&((dev)->mt76), __VA_ARGS__)
void mt76_mmio_init(struct mt76_dev *dev, void __iomem *regs);
+void mt76_mmio_disable_aspm(struct pci_dev *pdev);
static inline u16 mt76_chip(struct mt76_dev *dev)
{
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
b/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
index 6253ec5fbd72..06fb80163c8e 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
@@ -83,6 +83,8 @@ mt76pci_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
/* RG_SSUSB_CDR_BR_PE1D = 0x3 */
mt76_rmw_field(dev, 0x15c58, 0x3 << 6, 0x3);
+ mt76_mmio_disable_aspm(pdev);
+
return 0;
error:
--
2.21.0
>
> --
> Oleksandr Natalenko (post-factum)
signature.asc
Description: PGP signature
Re: mt76x2e hardware restart
> Hello. > > On 15.10.2019 18:52, Oleksandr Natalenko wrote: > > Thanks for the answer and the IRC discussion. As agreed I've applied > > [1] and [2], and have just swapped the card to try it again. So far, > > it works fine in 5 GHz band in 802.11ac mode as an AP. > > > > I'll give it more load with my phone over evening, and we can discuss > > what to do next (if needed) tomorrow again. Or feel free to drop me an > > email today. > > > > Thanks for your efforts. > > > > [1] > > https://github.com/LorenzoBianconi/wireless-drivers-next/commit/cf3436c42a297967235a9c9778620c585100529e.patch > > [2] > > https://github.com/LorenzoBianconi/wireless-drivers-next/commit/aad256eb62620f9646d39c1aa69234f50c89eed8.patch > > As agreed, here are iperf3 results, AP to STA distance is 2 meters. > > Client sends, TCP: > > [ ID] Interval Transfer Bitrate Retr > [ 5] 0.00-10.00 sec 70.4 MBytes 59.0 Mbits/sec 3800 > sender > [ 5] 0.00-10.03 sec 70.0 MBytes 58.6 Mbits/sec > receiver > > Client receives, TCP: > > [ ID] Interval Transfer Bitrate Retr > [ 5] 0.00-10.06 sec 196 MBytes 163 Mbits/sec 3081 > sender > [ 5] 0.00-10.01 sec 191 MBytes 160 Mbits/sec > receiver > > Client sends, UDP, 128 streams: > > [ ID] Interval Transfer Bitrate JitterLost/Total > Datagrams > [SUM] 0.00-10.00 sec 160 MBytes 134 Mbits/sec 0.000 ms 0/115894 > (0%) sender > [SUM] 0.00-10.01 sec 160 MBytes 134 Mbits/sec 0.347 ms 0/115892 > (0%) receiver > > Client receives, UDP, 128 streams: > > [ ID] Interval Transfer Bitrate JitterLost/Total > Datagrams > [SUM] 0.00-10.01 sec 119 MBytes 99.4 Mbits/sec 0.000 ms 0/85888 (0%) > sender > [SUM] 0.00-10.00 sec 119 MBytes 99.5 Mbits/sec 0.877 ms 0/85888 (0%) > receiver > > Given the HW is not the most powerful, the key point here is that nothing > crashed after doing these tests. Hi Oleksandr, thx a lot for testing these 2 patches. Now we need to understand why the chip hangs if we enable scatter gather dma transfer on x86 while it is working fine on multiple mips/arm devices (patch 2/2 just disable it for debugging). Regards, Lorenzo > > -- > Oleksandr Natalenko (post-factum) signature.asc Description: PGP signature
Re: [PATCH v3 4/5] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1
> On Sun, 28 Jul 2019 08:04:51 +0200 > Martin Kepplinger wrote: > > > On 27.07.19 19:48, Jonathan Cameron wrote: > > > On Thu, 25 Jul 2019 07:31:31 +0200 > > > Martin Kepplinger wrote: > > > > > >> The LSM9DS1's accelerometer / gyroscope unit and it's magnetometer > > >> (separately > > >> supported in iio/magnetometer/st_magn*) are located on a separate i2c > > >> addresses > > >> on the bus. > > >> > > >> For the datasheet, see > > >> https://www.st.com/resource/en/datasheet/lsm9ds1.pdf > > >> > > >> Treat it just like the LSM6* devices and, despite it's name, hook it up > > >> to the st_lsm6dsx driver, using it's basic functionality. > > >> > > >> Signed-off-by: Martin Kepplinger > > > I'm a little confused on this hardware. > > > > > > How does buffered output work if these are independently clocked? > > > > > > I took a quick look at the datasheet, and 'suspect' the answer is that > > > it runs at the gyro frequencies if both are enable. Is that right? > > > > > > > Thanks for reviewing, Jonathan, > > > > Correct. It says so in chapter 7.12. But that's a "problem" with all > > these imu devices, not specific to this addition right? > It's not a problem as such, but there is a related difference in this > device to the others supported by this driver. > > The other parts seem to allow for independent data rate setting, with > streaming to the buffer that isn't in 'lock step'. I.e you can get > > Ax_1, Ay_1, Az_1, Gx_1, Gy_1, Gz_1, Gx_2, Gy_2, Gz_2, Ax_2, Ay_2, Az_2, > Gy_3... correct > > That required us to split them up into two devices and means that, to fuse > data from these two source, userspace has to do the harder job of > aligning the two datasets. > > For this device, things are simpler in that you always a 'scan' that goes > across both accelerometer and gyroscope channels. That allows us to > represent it as a single IIO device with a single buffer. > > I'm not seeing any reference in the lsm9ds1 to the pattern registers > that are used to handle difference in frequency for the other > parts by letting us know what is actually present in each data set > in the fifo. > > Now, that doesn't meant we can't still handle them separately given > we already do that for other parts. what about reusing st_lsm6dsx_read_fifo() for lsm6ds0/lsm9ds1 but setting hw->sip to: - hw->sip = 1 (acc_sip = 1, gyro_sip = 0) when just the acc is enabled - hw->sip = 2 (acc_sip = 1, gyro_sip = 1) when both devices are enabled I guess it is just a matter of adding a 'bool fixed_pattern' in st_lsm6dsx_settings. What do you think? Regards, Lorenzo > > Anyhow, is my understanding correct? > > Jonathan > > > > > Sidenote: I thought about renaming things to "lsm6ds0" here just because > > of the name and because the registers are (almost) the same as for my > > lsm9ds1. But I'm not a fan of blindly doing that without being able to > > test. When the current patchset looks good to you, let's keep it that way. > > > > martin > signature.asc Description: PGP signature
Re: [PATCH] iio: imu: st_lsm6dsx: make IIO_CHAN_INFO_SCALE shared by type
> On Thu, 1 Aug 2019 16:39:08 +0200
> Martin Kepplinger wrote:
>
> > in_accel_x_scale, in_accel_y_scale and in_accel_z_scale are always
> > the same. The scale is still defined to be in "info_mask_separate".
> >
> > Userspace (iio-sensor-proxy and others) is not used to that and only
> > looks for "in_accel_scale" for the scaling factor to apply.
> >
> > Change IIO_CHAN_INFO_SCALE from being separate in all channel to be
> > shared by type.
> >
> > This removes in_accel_x_scale, in_accel_y_scale and in_accel_z_scale and
> > makes available in_accel_scale.
> >
> > Signed-off-by: Martin Kepplinger
> > ---
> >
> > AFAIK in all other drivers, IIO_CHAN_INFO_SCALE is "shared by type". Sure
> > devices are different, but LSM6DSX devices still don't have different
> > scales for x/y/z channels :)
>
> I'm fine with this, but would like a Lorenzo ack as we have had
> devices in other series where these are not equal. It used to
> be common in accelerometers as I think it was hard to get a large
> range in the vertical direction. Doubt that applies on these modern
> parts though!
>
> Thanks,
>
> Jonathan
AFAIK all the supported sensors have the same sensitivity on all axis and so I
am fine with this patch (it should be done in this way from day 0 actually :))
but is it going to break uapi? if not feel free to add:
Acked-by: Lorenzo Bianconi
Regards,
Lorenzo
>
>
> >
> > thanks,
> >
> > martin
> >
> >
> >
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > index af379a5429ed..59c3ab7cbb6f 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > @@ -56,8 +56,8 @@ enum st_lsm6dsx_hw_id {
> > .address = addr,\
> > .modified = 1, \
> > .channel2 = mod,\
> > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> > - BIT(IIO_CHAN_INFO_SCALE), \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> > .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> > .scan_index = scan_idx, \
> > .scan_type = { \
>
signature.asc
Description: PGP signature
Re: [PATCH v3 1/2] mt76: mt7615: enable support for mesh
>
> i tested your patch against a qca 9984 chipset using SAE and without
> encryption. both did not work. the devices are connecting, but no data
> connection is possible
Hi Sebastian,
I tested Ryder's patch using mt76x2 as mesh peer and it works fine for me.
Could you please provide some more info?
Regards,
Lorenzo
>
>
> Sebastian
>
> Am 03.06.2019 um 08:08 schrieb Ryder Lee:
> > Enable NL80211_IFTYPE_MESH_POINT and update its path.
> >
> > Signed-off-by: Ryder Lee
> > ---
> > Changes since v3 - fix a wrong expression
> > Changes since v2 - remove unused definitions
> > ---
> > drivers/net/wireless/mediatek/mt76/mt7615/init.c | 6 ++
> > drivers/net/wireless/mediatek/mt76/mt7615/main.c | 1 +
> > drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 4 +++-
> > drivers/net/wireless/mediatek/mt76/mt7615/mcu.h | 6 --
> > 4 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> > b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> > index 59f604f3161f..f860af6a42da 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> > @@ -133,6 +133,9 @@ static const struct ieee80211_iface_limit if_limits[] =
> > {
> > {
> > .max = MT7615_MAX_INTERFACES,
> > .types = BIT(NL80211_IFTYPE_AP) |
> > +#ifdef CONFIG_MAC80211_MESH
> > + BIT(NL80211_IFTYPE_MESH_POINT) |
> > +#endif
> >BIT(NL80211_IFTYPE_STATION)
> > }
> > };
> > @@ -195,6 +198,9 @@ int mt7615_register_device(struct mt7615_dev *dev)
> > dev->mt76.antenna_mask = 0xf;
> >
> > wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
> > +#ifdef CONFIG_MAC80211_MESH
> > + BIT(NL80211_IFTYPE_MESH_POINT) |
> > +#endif
> >BIT(NL80211_IFTYPE_AP);
> >
> > ret = mt76_register_device(&dev->mt76, true, mt7615_rates,
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> > b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> > index b0bb7cc12385..585e67fa2728 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> > @@ -37,6 +37,7 @@ static int get_omac_idx(enum nl80211_iftype type, u32
> > mask)
> >
> > switch (type) {
> > case NL80211_IFTYPE_AP:
> > + case NL80211_IFTYPE_MESH_POINT:
> > /* ap use hw bssid 0 and ext bssid */
> > if (~mask & BIT(HW_BSSID_0))
> > return HW_BSSID_0;
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > index 43f70195244c..e82297048449 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > @@ -754,6 +754,7 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
> >
> > switch (vif->type) {
> > case NL80211_IFTYPE_AP:
> > + case NL80211_IFTYPE_MESH_POINT:
> > tx_wlan_idx = mvif->sta.wcid.idx;
> > conn_type = CONNECTION_INFRA_AP;
> > break;
> > @@ -968,7 +969,7 @@ int mt7615_mcu_add_wtbl(struct mt7615_dev *dev, struct
> > ieee80211_vif *vif,
> > .rx_wtbl = {
> > .tag = cpu_to_le16(WTBL_RX),
> > .len = cpu_to_le16(sizeof(struct wtbl_rx)),
> > - .rca1 = vif->type != NL80211_IFTYPE_AP,
> > + .rca1 = vif->type == NL80211_IFTYPE_STATION,
> > .rca2 = 1,
> > .rv = 1,
> > },
> > @@ -1042,6 +1043,7 @@ static void sta_rec_convert_vif_type(enum
> > nl80211_iftype type, u32 *conn_type)
> > {
> > switch (type) {
> > case NL80211_IFTYPE_AP:
> > + case NL80211_IFTYPE_MESH_POINT:
> > if (conn_type)
> > *conn_type = CONNECTION_INFRA_STA;
> > break;
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> > b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> > index e96efb13fa4d..0915cb735699 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> > @@ -105,25 +105,19 @@ enum {
> > #define STA_TYPE_STABIT(0)
> > #define STA_TYPE_AP BIT(1)
> > #define STA_TYPE_ADHOC BIT(2)
> > -#define STA_TYPE_TDLSBIT(3)
> > #define STA_TYPE_WDSBIT(4)
> > #define STA_TYPE_BC BIT(5)
> >
> > #define NETWORK_INFRA BIT(16)
> > #define NETWORK_P2P BIT(17)
> > #define NETWORK_IBSSBIT(18)
> > -#define NETWORK_MESH BIT(19)
> > -#define NETWORK_BOW BIT(20)
> > #define NETWORK_WDS BIT(21)
> >
> > #define CONNECTION_INFRA_STA(STA_TYPE_STA | NETWORK_INFRA)
> > #define
Re: [PATCH v2 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
> For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
> spi and i2c mode.
>
> The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> them.
>
> Signed-off-by: Vitor Soares
> ---
Hi Vitor,
just a nit inline, but you can add my acked-by for st_lsm6dsx part in v3
Acked-by: Lorenzo Bianconi
Regards,
Lorenzo
> Changes in v2:
> Add support for LSM6DSR
> Set pm_ops to st_lsm6dsx_pm_ops
>
> drivers/iio/imu/st_lsm6dsx/Kconfig | 8 ++-
> drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 76
> +
> 3 files changed, 84 insertions(+), 1 deletion(-)
> create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig
> b/drivers/iio/imu/st_lsm6dsx/Kconfig
> index 002a423..8115936 100644
> --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> @@ -2,11 +2,12 @@
>
> config IIO_ST_LSM6DSX
> tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> - depends on (I2C || SPI)
> + depends on (I2C || SPI || I3C)
> select IIO_BUFFER
> select IIO_KFIFO_BUF
> select IIO_ST_LSM6DSX_I2C if (I2C)
> select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> + select IIO_ST_LSM6DSX_I3C if (I3C)
> help
> Say yes here to build support for STMicroelectronics LSM6DSx imu
> sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> @@ -24,3 +25,8 @@ config IIO_ST_LSM6DSX_SPI
> tristate
> depends on IIO_ST_LSM6DSX
> select REGMAP_SPI
> +
[...]
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "st_lsm6dsx.h"
> +
> +#define NAME_SIZE32
> +
> +struct st_lsm6dsx_i3c_data {
> + const char name[NAME_SIZE];
> + enum st_lsm6dsx_hw_id id;
> +};
> +
> +enum st_lsm6dsx_i3c_data_id {
> + ST_LSM6DSO_I3C_DATA_ID,
> + ST_LSM6DSR_I3C_DATA_ID,
> +};
do we really need them? maybe just use hw_data[n] adding a comment to indicate
the related sensor defining st_lsm6dsx_i3c_ids[]
> +
> +static const struct st_lsm6dsx_i3c_data hw_data[] = {
> + { ST_LSM6DSO_DEV_NAME, ST_LSM6DSO_ID },
> + { ST_LSM6DSR_DEV_NAME, ST_LSM6DSR_ID },
> +};
> +
> +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
> +{
> + const struct i3c_device_id *id = i3c_get_device_id(i3cdev);
> + const struct st_lsm6dsx_i3c_data *hw_data = id->data;
> + struct regmap *regmap;
> +
> + regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config);
> + if (IS_ERR(regmap)) {
> + dev_err(&i3cdev->dev, "Failed to register i3c regmap %d\n",
> + (int)PTR_ERR(regmap));
> + return PTR_ERR(regmap);
> + }
> +
> + return st_lsm6dsx_probe(&i3cdev->dev, 0, hw_data->id,
> + hw_data->name, regmap);
> +}
> +
> +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> + I3C_DEVICE(0x0104, 0x006C, &hw_data[ST_LSM6DSO_I3C_DATA_ID]),
> + I3C_DEVICE(0x0104, 0x006B, &hw_data[ST_LSM6DSR_I3C_DATA_ID]),
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> +
> +static struct i3c_driver st_lsm6dsx_driver = {
> + .driver = {
> + .name = "st_lsm6dsx_i3c",
> + .pm = &st_lsm6dsx_pm_ops,
> + },
> + .probe = st_lsm6dsx_i3c_probe,
> + .id_table = st_lsm6dsx_i3c_ids,
> +};
> +module_i3c_driver(st_lsm6dsx_driver);
> +
> +MODULE_AUTHOR("Vitor Soares ");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
signature.asc
Description: PGP signature
Re: [PATCH v2 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
> On Thu, 6 Jun 2019 17:12:04 +0200
> Vitor Soares wrote:
>
> > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
> > spi and i2c mode.
> >
> > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> > them.
> >
> > Signed-off-by: Vitor Soares
> > ---
> > Changes in v2:
> > Add support for LSM6DSR
> > Set pm_ops to st_lsm6dsx_pm_ops
> >
> > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 ++-
> > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 76
> > +
> > 3 files changed, 84 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> >
> > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig
> > b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > index 002a423..8115936 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > @@ -2,11 +2,12 @@
> >
> > config IIO_ST_LSM6DSX
> > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> > - depends on (I2C || SPI)
> > + depends on (I2C || SPI || I3C)
> > select IIO_BUFFER
> > select IIO_KFIFO_BUF
> > select IIO_ST_LSM6DSX_I2C if (I2C)
> > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> > + select IIO_ST_LSM6DSX_I3C if (I3C)
> > help
> > Say yes here to build support for STMicroelectronics LSM6DSx imu
> > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> > @@ -24,3 +25,8 @@ config IIO_ST_LSM6DSX_SPI
> > tristate
> > depends on IIO_ST_LSM6DSX
> > select REGMAP_SPI
> > +
> > +config IIO_ST_LSM6DSX_I3C
> > + tristate
> > + depends on IIO_ST_LSM6DSX
> > + select REGMAP_I3C
> > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile
> > b/drivers/iio/imu/st_lsm6dsx/Makefile
> > index 28cc673..57cbcd6 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/Makefile
> > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> > @@ -5,3 +5,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
> > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > new file mode 100644
> > index 000..70b70d1
> > --- /dev/null
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > @@ -0,0 +1,76 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > + *
> > + * Author: Vitor Soares
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "st_lsm6dsx.h"
> > +
> > +#define NAME_SIZE 32
> > +
> > +struct st_lsm6dsx_i3c_data {
> > + const char name[NAME_SIZE];
>
> I think I mentioned already that you can simply have
>
> const char *name;
>
> > + enum st_lsm6dsx_hw_id id;
> > +};
> > +
> > +enum st_lsm6dsx_i3c_data_id {
> > + ST_LSM6DSO_I3C_DATA_ID,
> > + ST_LSM6DSR_I3C_DATA_ID,
> > +};
> > +
> > +static const struct st_lsm6dsx_i3c_data hw_data[] = {
> > + { ST_LSM6DSO_DEV_NAME, ST_LSM6DSO_ID },
> > + { ST_LSM6DSR_DEV_NAME, ST_LSM6DSR_ID },
> > +};
> > +
> > +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > +};
> > +
> > +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
> > +{
> > + const struct i3c_device_id *id = i3c_get_device_id(i3cdev);
> > + const struct st_lsm6dsx_i3c_data *hw_data = id->data;
> > + struct regmap *regmap;
> > +
> > + regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config);
> > + if (IS_ERR(regmap)) {
> > + dev_err(&i3cdev->dev, "Failed to register i3c regmap %d\n",
> > + (int)PTR_ERR(regmap));
> > + return PTR_ERR(regmap);
> > + }
> > +
> > + return st_lsm6dsx_probe(&i3cdev->dev, 0, hw_data->id,
> > + hw_data->name, regmap);
> > +}
> > +
> > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > + I3C_DEVICE(0x0104, 0x006C, &hw_data[ST_LSM6DSO_I3C_DATA_ID]),
> > + I3C_DEVICE(0x0104, 0x006B, &hw_data[ST_LSM6DSR_I3C_DATA_ID]),
>
> Still find that form counter-intuitive since you'd have to first go
> look at what's the value of ST_LSM6DSO_I3C_DATA_ID, then go check the
> entry in hw_data to find what's in there. Too many ways to get things
> wrong IMHO.
>
> The following form would make it much more obvious/easy to follow:
>
> static const st_lsm6dsx_i3c_data st_lsm6dso_i3c_data = {
> ST_LSM6DSO_DEV_NAME, ST_LSM6DSO_ID,
> };
>
> static const st_lsm6dsx_i3c_data st_lsm6dsr_i3c_data = {
> ST_LSM6DSR_DEV_NAME, ST_LSM6DSR_ID,
> };
>
> static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> I3C_DEVICE(0x0104, 0x006C, &st_lsm6dso_i3c_data),
> I3C_DEVI
Re: linux-next: Fixes tag needs some work in the wireless-drivers-next tree
> Hi all,
>
> In commit
>
> d923cf6bc38a ("mt76: mt7615: fix sparse warnings: warning: cast from
> restricted __le16")
>
> Fixes tag
>
> Fixes: 3ca0a6f6e9df ("mt7615: mcu: use standard signature for
> mt7615_mcu_msg_send")
>
> has these problem(s):
>
> - Target SHA1 does not exist
>
> Did you mean
>
> Fixes: 516c3e380533 ("mt7615: mcu: use standard signature for
> mt7615_mcu_msg_send")
>
> In commit
>
> eda96044de27 ("mt76: mt7615: fix sparse warnings: incorrect type in
> assignment (different base types)")
>
> Fixes tag
>
> Fixes: 7339fbc0caa5 ("mt7615: mcu: do not use function pointers whenever
> possible")
>
> has these problem(s):
>
> - Target SHA1 does not exist
>
> Did you mean
>
> Fixes: 1ca8089a55ee ("mt7615: mcu: do not use function pointers whenever
> possible")
>
> In commit
>
> 1a09d9e0e5f0 ("mt76: mt7615: fix incorrect settings in mesh mode")
>
> Fixes tag
>
> Fixes: f072c7ba2150 ("mt76: mt7615: enable support for mesh")
>
> has these problem(s):
>
> - Target SHA1 does not exist
>
> Did you mean
>
> Fixes: f4ec7fdf7f83 ("mt76: mt7615: enable support for mesh")
Hi Stephen,
I used the hashes from my local git tree that are different from upstream ones,
sorry for the inconvenience. I will pay more attention next time.
Regards,
Lorenzo
>
> --
> Cheers,
> Stephen Rothwell
signature.asc
Description: PGP signature
Re: [PATCH v3 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
> For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
> spi and i2c mode.
>
> The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> them.
Hi Vitor,
just few comments inline.
Regards,
Lorenzo
>
> Signed-off-by: Vitor Soares
> Acked-by: Lorenzo Bianconi
> ---
> Changes in v3:
> Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name
> Use st_lsm6dsx_probe new form
>
> Changes in v2:
> Add support for LSM6DSR
> Set pm_ops to st_lsm6dsx_pm_ops
>
> drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 61
> +
> 3 files changed, 69 insertions(+), 1 deletion(-)
> create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
>
[...]
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> new file mode 100644
> index 000..f683754
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> @@ -0,0 +1,61 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> + *
> + * Author: Vitor Soares
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "st_lsm6dsx.h"
> +
> +static const struct i3c_device_id st_lsm6dsx_i3c_ids[];
> +
why do we need this? I guess you can just move st_lsm6dsx_i3c_ids definition
here
> +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
> +{
> + const struct i3c_device_id *id = i3c_device_match_id(i3cdev,
> + st_lsm6dsx_i3c_ids);
i3c_device_match_id can theoretically fail so is it better to check
return value here? (maybe I am too paranoid :))
> + struct regmap *regmap;
> + int hw_id = (int)id->data;
I guess we do not need this since we use it just in st_lsm6dsx_probe(),
we can just do:
return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap);
> +
> + regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config);
> + if (IS_ERR(regmap)) {
> + dev_err(&i3cdev->dev, "Failed to register i3c regmap %d\n",
> + (int)PTR_ERR(regmap));
> + return PTR_ERR(regmap);
> + }
> +
> + return st_lsm6dsx_probe(&i3cdev->dev, 0, hw_id, regmap);
> +}
> +
> +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID),
> + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID),
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> +
> +static struct i3c_driver st_lsm6dsx_driver = {
> + .driver = {
> + .name = "st_lsm6dsx_i3c",
> + .pm = &st_lsm6dsx_pm_ops,
> + },
> + .probe = st_lsm6dsx_i3c_probe,
> + .id_table = st_lsm6dsx_i3c_ids,
> +};
> +module_i3c_driver(st_lsm6dsx_driver);
> +
> +MODULE_AUTHOR("Vitor Soares ");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
signature.asc
Description: PGP signature
Re: [PATCH v3 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
> From: Vitor Soares
> Date: Thu, Jul 11, 2019 at 11:12:34
>
> > Hi Lorenzo,
> >
> > From: Lorenzo Bianconi
> > Date: Wed, Jul 10, 2019 at 20:44:05
> >
> > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only
> > > > in
> > > > spi and i2c mode.
> > > >
> > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> > > > them.
> > >
> > > Hi Vitor,
> > >
> > > just few comments inline.
> > >
> > > Regards,
> > > Lorenzo
> > >
> > > >
> > > > Signed-off-by: Vitor Soares
> > > > Acked-by: Lorenzo Bianconi
> > > > ---
> > > > Changes in v3:
> > > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name
> > > > Use st_lsm6dsx_probe new form
> > > >
> > > > Changes in v2:
> > > > Add support for LSM6DSR
> > > > Set pm_ops to st_lsm6dsx_pm_ops
> > > >
> > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 61
> > > > +
> > > > 3 files changed, 69 insertions(+), 1 deletion(-)
> > > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > >
> > >
> > > [...]
> > > > +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
> > > > +{
> > > > + const struct i3c_device_id *id = i3c_device_match_id(i3cdev,
> > > > +
> > > > st_lsm6dsx_i3c_ids);
> > >
> > > i3c_device_match_id can theoretically fail so is it better to check
> > > return value here? (maybe I am too paranoid :))
>
> I was preparing the patch and if the i3c_device_match_id() fail it return
> NULL so the st_lsm6dsx_probe() will fail automatically.
> Checking the spi_get_device_id(), the drivers don't test the return value
> too.
multiple drivers deference it directly so I am fine to skip this check.
Regards,
Lorenzo
>
> Do you think it is really necessary to test it before the
> st_lsm6dsx_probe() function?
>
> Best regards,
> Vitor Soares
signature.asc
Description: PGP signature
Re: MT76x2U crashes XHCI driver on AMD Ryzen system
> (cc: IOMMU & page_frag_alloc maintainers) > > On Tue, Jan 15, 2019 at 10:04:01AM +0100, Lorenzo Bianconi wrote: > > > On Mon, Jan 14, 2019 at 1:18 AM Lorenzo Bianconi > > > wrote: > > > > > > > > > On Sun, Jan 13, 2019 at 11:00 AM Lorenzo Bianconi > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sun, Jan 13, 2019 at 5:33 AM, Lorenzo Bianconi > > > > > > > wrote: > > > > > > > > > > > > > > Direct. No VM used. This is the only peripheral causing this > > > > > > > issue. > > > > > > > > > > > > > > Is the device connected to a usb3.0 port? If so, could you please > > > > > > > try to connect the dongle to a 2.0 one? > > > > > > > > > > > > > > I tried through a USB 2.0 port. Shouldn't make a difference as > > > > > > > they both use the xhci driver. > > > > > > > > > > > > > > > > > > > mt76x2u supports scatter-gather on usb 3.0 (not on 2.0) > > > > > Tried a USB 3 port. Same result. > > > > > > > > > > > > > Could you please double check if IOMMU is enabled? > > > > > > > > > > > > > > > > > > > Have you tried to disable it? Does it make any difference? > > > > > No idea how. UEFI doesn't seem to show anything similar. > > > > > > > > > > Similar bug report: https://bugzilla.kernel.org/show_bug.cgi?id=202241 > > FWIW: I provided some patches in the bugzilla, which were reported to > solve the problem. But I looking for confirmation if both are needed: > > 0001-mt76x02u-use-usb_bulk_msg-to-upload-firmware.patch > 0002-mt76usb-do-not-use-compound-head-page-for-SG-I-O.patch > > Or problem can be solved by just one of it (either first or second). > > Additionally I'm not 100% sure if > > 0002-mt76usb-do-not-use-compound-head-page-for-SG-I-O.patch > > is correct. So perhaps some IOMMU maintainer could look at it. > > > > > You should be able to disable iommu using GRUB_CMDLINE_LINUX in > > > > /etc/default/grub (I guess setting iommu=off and reinstalling grub) > > > > https://wiki.gentoo.org/wiki/IOMMU_SWIOTLB > > > Yep. Working great now. I wonder what mt76 is doing to cause the crash > > > though... > > > > Thanks for bisecting the issue. > > Lorenzo, what you mean by 'bisecting' here ? Someone did 'git bisect' > on this issue? > Hi Stanislaw, I was meaning 'help bisecting' the issue > > I think amd iommu does not support well usb scatter-gather > > (used by default in mt76u). I am working on a series in order to add the > > possibility to > > disable it. > > Even if that true that AMD IOMMU does not support 'well' SG (what I think > is not true) disabling SG in mt76 driver is not right solution. Right > solution would be propagate the issue to AMD IOMMU maintainers > (already CCed). I meant that AMD iommu seems to have different constraints respect to Intel one. Regards, Lorenzo > > One problem in mt76 is page_frag_alloc() usage with different sizes. > page_frag_alloc() unlike like other allocators do not assure alignment > and relay on callers to provide buffers sizes that are aligned. > Unaligned buffer might then not be appropriate for DMA. > > Another issue is that dma_map_sg() & dma_map_page() may require some > constraints. I'm not sure about that and I want to clarify that with > CCed mm maintainers. I think DMA drivers may expect sg->offset < PAGE_SIZE > for both dma_map_sg() and dma_map_page(). Additionally dma_map_page() > maight expect that offset & length specify buffer within one page. > > Stanislaw
Re: mt7612 suspend/resume issue
> Hello, Lorenzo et al.
Hi Oleksandr,
>
> I'm using MT7612 mini-PCIE cards as both AP in a home server and as a client
> in
> a laptop. The AP works perfectly (after some fixing from your side; thanks for
> that!), and so does the client modulo it has issues during system resume.
>
[...]
>
> The Wi-Fi becomes unusable from this point. If I `modprobe -r` the "mt76x2e"
> module
> after this splat, the system hangs completely.
>
> If the system resumes fine, the resume itself takes quite some time (more than
> 10 seconds).
>
> I've found a workaround for this, though. It seems the system behaves fine if
> I
> do `modprobe -r mt76x2e` before it goes to sleep, and then `modprobe mt76x2e`
> after resume. Also, the resume time improves greatly.
>
> I cannot say if it is some regression or not. I've installed the card
> just recently, and used it with v5.7 kernel series only.
>
> Do you have any idea what could go wrong and how to approach the issue?
I have not reproduced the issue myself yet, but I guess we can try:
1- update to latest Felix's tree [1]
2- could you please try to apply the patch below? (compile-test only)
Regards,
Lorenzo
[1] https://github.com/nbd168/wireless
From 400268a0ee5843cf736308504dfbd2f20a291eaf Mon Sep 17 00:00:00 2001
Message-Id:
<400268a0ee5843cf736308504dfbd2f20a291eaf.1592478809.git.lore...@kernel.org>
From: Lorenzo Bianconi
Date: Thu, 18 Jun 2020 13:10:11 +0200
Subject: [PATCH] mt76: mt76x2: fix pci suspend
Signed-off-by: Lorenzo Bianconi
---
.../net/wireless/mediatek/mt76/mt76x02_dma.h | 1 +
.../net/wireless/mediatek/mt76/mt76x02_mmio.c | 15 +
.../net/wireless/mediatek/mt76/mt76x2/pci.c | 58 +++
3 files changed, 74 insertions(+)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_dma.h
b/drivers/net/wireless/mediatek/mt76/mt76x02_dma.h
index 4aff4f8e87b6..6262f2ecded5 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_dma.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_dma.h
@@ -62,5 +62,6 @@ mt76x02_wait_for_wpdma(struct mt76_dev *dev, int timeout)
int mt76x02_dma_init(struct mt76x02_dev *dev);
void mt76x02_dma_disable(struct mt76x02_dev *dev);
void mt76x02_dma_cleanup(struct mt76x02_dev *dev);
+void mt76x02_dma_reset(struct mt76x02_dev *dev);
#endif /* __MT76x02_DMA_H */
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
index cbbe986655fe..e2631c964331 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
@@ -348,6 +348,21 @@ void mt76x02_dma_disable(struct mt76x02_dev *dev)
}
EXPORT_SYMBOL_GPL(mt76x02_dma_disable);
+void mt76x02_dma_reset(struct mt76x02_dev *dev)
+{
+ int i;
+
+ mt76x02_dma_disable(dev);
+ usleep_range(1000, 2000);
+
+ for (i = 0; i < __MT_TXQ_MAX; i++)
+ mt76_queue_tx_cleanup(dev, i, true);
+ mt76_for_each_q_rx(&dev->mt76, i)
+ mt76_queue_rx_reset(dev, i);
+
+ mt76x02_dma_enable(dev);
+}
+
void mt76x02_mac_start(struct mt76x02_dev *dev)
{
mt76x02_mac_reset_counters(dev);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
b/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
index 53ca0cedf026..5543e242fb9b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
@@ -103,6 +103,60 @@ mt76pci_remove(struct pci_dev *pdev)
mt76_free_device(mdev);
}
+static int __maybe_unused
+mt76x2e_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+ struct mt76_dev *mdev = pci_get_drvdata(pdev);
+ struct mt76x02_dev *dev = container_of(mdev, struct mt76x02_dev, mt76);
+ int i, err;
+
+ napi_disable(&mdev->tx_napi);
+ tasklet_kill(&mdev->pre_tbtt_tasklet);
+ tasklet_kill(&mdev->tx_tasklet);
+
+ mt76_for_each_q_rx(mdev, i)
+ napi_disable(&mdev->napi[i]);
+
+ mt76x02_dma_reset(dev);
+
+ pci_enable_wake(pdev, pci_choose_state(pdev, state), true);
+ pci_save_state(pdev);
+ err = pci_set_power_state(pdev, pci_choose_state(pdev, state));
+ if (err)
+ goto restore;
+
+ return 0;
+
+restore:
+ mt76_for_each_q_rx(mdev, i)
+ napi_enable(&mdev->napi[i]);
+ napi_enable(&mdev->tx_napi);
+
+ return err;
+}
+
+static int __maybe_unused
+mt76x2e_resume(struct pci_dev *pdev)
+{
+ struct mt76_dev *mdev = pci_get_drvdata(pdev);
+ int i, err;
+
+ err = pci_set_power_state(pdev, PCI_D0);
+ if (err)
+ return err;
+
+ pci_restore_state(pdev);
+
+ mt76_for_each_q_rx(mdev, i) {
+ napi_enable(&mdev->napi[i]);
+ napi_schedule(&mdev->napi[i]);
+ }
+
Re: mt7612 suspend/resume issue
> Hello, Lorenzo.
>
> On Sun, Jun 21, 2020 at 10:54 PM Lorenzo Bianconi wrote:
> > > > +static int __maybe_unused
> > > > +mt76x2e_suspend(struct pci_dev *pdev, pm_message_t state)
> > > > +{
> > > > + struct mt76_dev *mdev = pci_get_drvdata(pdev);
> > > > + struct mt76x02_dev *dev = container_of(mdev, struct mt76x02_dev,
> > > > mt76);
> > > > + int i, err;
> >
> > can you please double-check what is the PCI state requested during suspend?
>
> Do you mean ACPI S3 (this is the state the system enters)? If not,
> what should I check and where?
yes, right. Just for debugging, can you please force the card in PCI_D0 during
the
suspend?
Regards,
Lorenzo
>
> Thanks.
>
> --
> Best regards,
> Oleksandr Natalenko (post-factum)
> Principal Software Maintenance Engineer
>
signature.asc
Description: PGP signature
Re: mt7612 suspend/resume issue
> On Mon, Jun 22, 2020 at 4:53 PM Lorenzo Bianconi
> wrote:
> > > On Sun, Jun 21, 2020 at 10:54 PM Lorenzo Bianconi
> > > wrote:
> > > > > > +static int __maybe_unused
> > > > > > +mt76x2e_suspend(struct pci_dev *pdev, pm_message_t state)
> > > > > > +{
> > > > > > + struct mt76_dev *mdev = pci_get_drvdata(pdev);
> > > > > > + struct mt76x02_dev *dev = container_of(mdev, struct
> > > > > > mt76x02_dev, mt76);
> > > > > > + int i, err;
> > > >
> > > > can you please double-check what is the PCI state requested during
> > > > suspend?
> > >
> > > Do you mean ACPI S3 (this is the state the system enters)? If not,
> > > what should I check and where?
> >
> > yes, right. Just for debugging, can you please force the card in PCI_D0
> > during the
> > suspend?
>
> Do you want me to do this:
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
> b/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
> index 5543e242fb9b..e558342cce03 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
> @@ -119,9 +119,8 @@ mt76x2e_suspend(struct pci_dev *pdev, pm_message_t state)
>
> mt76x02_dma_reset(dev);
>
> -pci_enable_wake(pdev, pci_choose_state(pdev, state), true);
> pci_save_state(pdev);
> -err = pci_set_power_state(pdev, pci_choose_state(pdev, state));
> +err = pci_set_power_state(pdev, PCI_D0);
> if (err)
> goto restore;
I think you can just substitute pci_choose_state(pdev, state) with PCI_D0, not
removing pci_enable_wake()
Regards,
Lorenzo
>
> ?
>
> --
> Best regards,
> Oleksandr Natalenko (post-factum)
> Principal Software Maintenance Engineer
>
signature.asc
Description: PGP signature
Re: [PATCH] iio: imu: st_lsm6dsx: Scaling factor type set to IIO_VAL_INT_PLUS_NANO
> From: Mario Tesi
>
> Scaling factor values for Acc lead to an unacceptable rounding of the
> full scale (FS) calculated by some SensorHAL on Android devices. For examples
> setting FS to 4g the in_accel_x_scale, in_accel_y_scale and in_accel_z_scale
> are 0.001196 on 6 decimal digits and the FS is
> 0.001196 × ((2^15) − 1) ~= 39.1893 m/s^2.
>
> Android CTS R10 SensorParameterRangeTest test expects a value greater than
> 39.20 m/s^2 so this test fails (ACCELEROMETER_MAX_RANGE = 4 * 9.80).
>
> Using 9 decimal digits the new scale factor is 0.001196411 and the FS now
> is 0.001196411 × ((2^15)−1) ~= 39.2028 m/s^2.
>
> This patch extends to IIO_VAL_INT_PLUS_NANO type the scaling factor to all
> IMU devices where SensorParameterRangeTest CTS test fails.
>
> Signed-off-by: Mario Tesi
Hi Mario,
just a minor comment inline. Fixing it:
Acked-by: Lorenzo Bianconi
> ---
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 136
> ---
> 1 file changed, 79 insertions(+), 57 deletions(-)
>
[...]
> fs_table = &hw->settings->fs_table[sensor->id];
> for (i = 0; i < fs_table->fs_len; i++)
> - len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
> + len += scnprintf(buf + len, PAGE_SIZE - len, "0.%09u ",
>fs_table->fs_avl[i].gain);
> buf[len - 1] = '\n';
>
> return len;
> }
>
> +static int st_lsm6dsx_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_ANGL_VEL:
> + case IIO_ACCEL:
> + return IIO_VAL_INT_PLUS_NANO;
> + default:
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> + default:
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> +
> + return -EINVAL;
you can remove this
> +}
> +
> static
> IIO_DEV_ATTR_SAMP_FREQ_AVAIL(st_lsm6dsx_sysfs_sampling_frequency_avail);
> static IIO_DEVICE_ATTR(in_accel_scale_available, 0444,
> st_lsm6dsx_sysfs_scale_avail, NULL, 0);
> @@ -1868,6 +1888,7 @@ static const struct iio_info st_lsm6dsx_acc_info = {
> .read_event_config = st_lsm6dsx_read_event_config,
> .write_event_config = st_lsm6dsx_write_event_config,
> .hwfifo_set_watermark = st_lsm6dsx_set_watermark,
> + .write_raw_get_fmt = st_lsm6dsx_write_raw_get_fmt,
> };
>
> static struct attribute *st_lsm6dsx_gyro_attributes[] = {
> @@ -1885,6 +1906,7 @@ static const struct iio_info st_lsm6dsx_gyro_info = {
> .read_raw = st_lsm6dsx_read_raw,
> .write_raw = st_lsm6dsx_write_raw,
> .hwfifo_set_watermark = st_lsm6dsx_set_watermark,
> + .write_raw_get_fmt = st_lsm6dsx_write_raw_get_fmt,
> };
>
> static int st_lsm6dsx_get_drdy_pin(struct st_lsm6dsx_hw *hw, int *drdy_pin)
> --
> 2.7.4
>
signature.asc
Description: PGP signature
Re: [PATCH 2/2] mt76: mt7615: update peer's bssid when state transition changes
> Driver should update peer's bssid and bss information when
> state transition changes.
>
> Signed-off-by: Ryder Lee
> ---
> .../net/wireless/mediatek/mt76/mt7615/main.c | 5 +-
> .../net/wireless/mediatek/mt76/mt7615/mcu.c | 49 ++-
> 2 files changed, 27 insertions(+), 27 deletions(-)
>
[...]
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> index e82086eb8aa4..8fc12cd37906 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> @@ -741,17 +741,6 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
> u8 *buf, *data, tx_wlan_idx = 0;
> struct req_hdr *hdr;
>
> - if (en) {
> - len += sizeof(struct bss_info_omac);
> - features |= BIT(BSS_INFO_OMAC);
> - if (mvif->omac_idx > EXT_BSSID_START) {
> - len += sizeof(struct bss_info_ext_bss);
> - features |= BIT(BSS_INFO_EXT_BSS);
> - ntlv++;
> - }
> - ntlv++;
> - }
> -
> switch (vif->type) {
> case NL80211_IFTYPE_AP:
> case NL80211_IFTYPE_MESH_POINT:
> @@ -759,22 +748,23 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
> conn_type = CONNECTION_INFRA_AP;
> break;
> case NL80211_IFTYPE_STATION: {
> - struct ieee80211_sta *sta;
> - struct mt7615_sta *msta;
> -
> - rcu_read_lock();
> -
> - sta = ieee80211_find_sta(vif, vif->bss_conf.bssid);
> - if (!sta) {
> + /* TODO: enable BSS_INFO_UAPSD & BSS_INFO_PM */
> + if (en) {
> + struct ieee80211_sta *sta;
> + struct mt7615_sta *msta;
> +
> + rcu_read_lock();
> + sta = ieee80211_find_sta(vif, vif->bss_conf.bssid);
> + if (!sta) {
> + rcu_read_unlock();
> + return -EINVAL;
> + }
> +
> + msta = (struct mt7615_sta *)sta->drv_priv;
> + tx_wlan_idx = msta->wcid.idx;
> rcu_read_unlock();
> - return -EINVAL;
> }
> -
> - msta = (struct mt7615_sta *)sta->drv_priv;
> - tx_wlan_idx = msta->wcid.idx;
> conn_type = CONNECTION_INFRA_STA;
> -
> - rcu_read_unlock();
> break;
> }
> default:
> @@ -782,6 +772,17 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
> break;
> }
>
> + if (en) {
> + len += sizeof(struct bss_info_omac);
> + features |= BIT(BSS_INFO_OMAC);
> + if (mvif->omac_idx > EXT_BSSID_START) {
> + len += sizeof(struct bss_info_ext_bss);
> + features |= BIT(BSS_INFO_EXT_BSS);
> + ntlv++;
> + }
> + ntlv++;
> + }
What did you move this chunk down?
Regards,
Lorenzo
> +
> buf = kzalloc(len, GFP_KERNEL);
> if (!buf)
> return -ENOMEM;
> --
> 2.18.0
>
signature.asc
Description: PGP signature
Re: [PATCH v2] mt76: mt7615: add support for per-chain signal strength reporting
> Fill in RX status->chain_signal to avoid empty value.
>
> Signed-off-by: Ryder Lee
> ---
> Changes since v2 - correct calculation sequence
> ---
> .../net/wireless/mediatek/mt76/mt7615/mac.c | 30 ++-
> .../net/wireless/mediatek/mt76/mt7615/mac.h | 5
> 2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mac.c
> b/drivers/net/wireless/mediatek/mt76/mt7615/mac.c
> index b60d42b5923d..2f49a99e77b1 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mac.c
> @@ -13,6 +13,11 @@
> #include "../dma.h"
> #include "mac.h"
>
[...]
> @@ -169,7 +175,29 @@ int mt7615_mac_fill_rx(struct mt7615_dev *dev, struct
> sk_buff *skb)
>
> status->enc_flags |= RX_ENC_FLAG_STBC_MASK * stbc;
>
> - /* TODO: RSSI */
> + status->chains = dev->mt76.antenna_mask;
> + status->chain_signal[0] = to_rssi(MT_RXV4_RCPI0, rxdg3);
> + status->chain_signal[1] = to_rssi(MT_RXV4_RCPI1, rxdg3);
> + status->chain_signal[2] = to_rssi(MT_RXV4_RCPI2, rxdg3);
> + status->chain_signal[3] = to_rssi(MT_RXV4_RCPI3, rxdg3);
> + status->signal = status->chain_signal[0];
> +
> + switch (status->chains) {
> + case 0xf:
> + status->signal = max(status->signal,
> + status->chain_signal[3]);
> + /* fall through */
> + case 0x7:
> + status->signal = max(status->signal,
> + status->chain_signal[2]);
> + /* fall through */
> + case 0x3:
> + status->signal = max(status->signal,
> + status->chain_signal[1]);
> + break;
> + default:
> + break;
> + }
is it possible to enable rx chains selectively (e.g. chain 0 and 2)?
If so we can do something like:
for (i = 1; i < hweight8(dev->mt76.antenna_mask); i++) {
if (!(status->chains & BIT(i)))
continue;
status->signal = max(status->signal,
status->chain_signal[i]);
}
Regards,
Lorenzo
> rxd += 6;
> if ((u8 *)rxd - skb->data >= skb->len)
> return -EINVAL;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mac.h
> b/drivers/net/wireless/mediatek/mt76/mt7615/mac.h
> index 18ad4b8a3807..b00ce8db58e9 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mac.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mac.h
> @@ -98,6 +98,11 @@ enum rx_pkt_type {
> #define MT_RXV2_GROUP_ID GENMASK(26, 21)
> #define MT_RXV2_LENGTH GENMASK(20, 0)
>
> +#define MT_RXV4_RCPI3GENMASK(31, 24)
> +#define MT_RXV4_RCPI2GENMASK(23, 16)
> +#define MT_RXV4_RCPI1GENMASK(15, 8)
> +#define MT_RXV4_RCPI0GENMASK(7, 0)
> +
> enum tx_header_format {
> MT_HDR_FORMAT_802_3,
> MT_HDR_FORMAT_CMD,
> --
> 2.18.0
>
signature.asc
Description: PGP signature
Re: [PATCH 1/2] mt76: mt7615: enable support for mesh
> Enable NL80211_IFTYPE_MESH_POINT and add its path.
>
> Signed-off-by: Ryder Lee
> ---
> drivers/net/wireless/mediatek/mt76/mt7615/init.c | 6 ++
> drivers/net/wireless/mediatek/mt76/mt7615/main.c | 1 +
> drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 5 -
> 3 files changed, 11 insertions(+), 1 deletion(-)
>
[...]
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> index b0bb7cc12385..585e67fa2728 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> @@ -37,6 +37,7 @@ static int get_omac_idx(enum nl80211_iftype type, u32 mask)
>
> switch (type) {
> case NL80211_IFTYPE_AP:
> + case NL80211_IFTYPE_MESH_POINT:
> /* ap use hw bssid 0 and ext bssid */
> if (~mask & BIT(HW_BSSID_0))
> return HW_BSSID_0;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> index 43f70195244c..8b8db526cb16 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> @@ -754,6 +754,7 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
>
> switch (vif->type) {
> case NL80211_IFTYPE_AP:
> + case NL80211_IFTYPE_MESH_POINT:
> tx_wlan_idx = mvif->sta.wcid.idx;
> conn_type = CONNECTION_INFRA_AP;
Just out of curiosity, why not using CONNECTION_MESH_{AP,STA} here?
why not NETWORK_MESH?
> break;
> @@ -968,7 +969,8 @@ int mt7615_mcu_add_wtbl(struct mt7615_dev *dev, struct
> ieee80211_vif *vif,
> .rx_wtbl = {
> .tag = cpu_to_le16(WTBL_RX),
> .len = cpu_to_le16(sizeof(struct wtbl_rx)),
> - .rca1 = vif->type != NL80211_IFTYPE_AP,
> + .rca1 = vif->type != (NL80211_IFTYPE_AP ||
> + NL80211_IFTYPE_MESH_POINT),
> .rca2 = 1,
> .rv = 1,
> },
> @@ -1042,6 +1044,7 @@ static void sta_rec_convert_vif_type(enum
> nl80211_iftype type, u32 *conn_type)
> {
> switch (type) {
> case NL80211_IFTYPE_AP:
> + case NL80211_IFTYPE_MESH_POINT:
> if (conn_type)
> *conn_type = CONNECTION_INFRA_STA;
> break;
same here.
Regards,
Lorenzo
> --
> 2.18.0
>
signature.asc
Description: PGP signature
Re: mt7612 suspend/resume issue
> Hello, Lorenzo.
Hi Oleksandr,
>
> Thanks for the quick reply. Please see my observation below.
>
> On Thu, Jun 18, 2020 at 01:18:59PM +0200, Lorenzo Bianconi wrote:
> > I have not reproduced the issue myself yet, but I guess we can try:
> > 1- update to latest Felix's tree [1]
> > 2- could you please try to apply the patch below? (compile-test only)
>
> I've started with trying your patch first (apllied to v5.7.4). Please
> see my comments on it inline.
>
> > [1] https://github.com/nbd168/wireless
> >
> > From 400268a0ee5843cf736308504dfbd2f20a291eaf Mon Sep 17 00:00:00 2001
> > Message-Id:
> > <400268a0ee5843cf736308504dfbd2f20a291eaf.1592478809.git.lore...@kernel.org>
> > From: Lorenzo Bianconi
> > Date: Thu, 18 Jun 2020 13:10:11 +0200
> > Subject: [PATCH] mt76: mt76x2: fix pci suspend
> >
> > Signed-off-by: Lorenzo Bianconi
> > ---
[...]
> > + for (i = 0; i < __MT_TXQ_MAX; i++)
> > + mt76_queue_tx_cleanup(dev, i, true);
> > + mt76_for_each_q_rx(&dev->mt76, i)
>
> Since v5.7.4 doesn't have this macro, I've pulled it manually.
this is why I asked to use Felix's tree :)
>
> > + mt76_queue_rx_reset(dev, i);
> > +
> > + mt76x02_dma_enable(dev);
> > +}
>
> I had to add EXPORT_SYMBOL_GPL(mt76x02_dma_reset) in order to get the
> kernel linked successfully.
ack, sorry
>
> > +
> > void mt76x02_mac_start(struct mt76x02_dev *dev)
> > {
> > mt76x02_mac_reset_counters(dev);
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
> > b/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
> > index 53ca0cedf026..5543e242fb9b 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
> > @@ -103,6 +103,60 @@ mt76pci_remove(struct pci_dev *pdev)
> > mt76_free_device(mdev);
> > }
> >
> > +static int __maybe_unused
> > +mt76x2e_suspend(struct pci_dev *pdev, pm_message_t state)
> > +{
> > + struct mt76_dev *mdev = pci_get_drvdata(pdev);
> > + struct mt76x02_dev *dev = container_of(mdev, struct mt76x02_dev, mt76);
> > + int i, err;
can you please double-check what is the PCI state requested during suspend?
Regards,
Lorenzo
> > +
> > + napi_disable(&mdev->tx_napi);
> > + tasklet_kill(&mdev->pre_tbtt_tasklet);
> > + tasklet_kill(&mdev->tx_tasklet);
> > +
> > + mt76_for_each_q_rx(mdev, i)
> > + napi_disable(&mdev->napi[i]);
> > +
> > + mt76x02_dma_reset(dev);
> > +
> > + pci_enable_wake(pdev, pci_choose_state(pdev, state), true);
> > + pci_save_state(pdev);
> > + err = pci_set_power_state(pdev, pci_choose_state(pdev, state));
> > + if (err)
> > + goto restore;
> > +
> > + return 0;
> > +
> > +restore:
> > + mt76_for_each_q_rx(mdev, i)
> > + napi_enable(&mdev->napi[i]);
> > + napi_enable(&mdev->tx_napi);
> > +
> > + return err;
> > +}
> > +
> > +static int __maybe_unused
> > +mt76x2e_resume(struct pci_dev *pdev)
> > +{
> > + struct mt76_dev *mdev = pci_get_drvdata(pdev);
> > + int i, err;
> > +
> > + err = pci_set_power_state(pdev, PCI_D0);
> > + if (err)
> > + return err;
> > +
> > + pci_restore_state(pdev);
> > +
> > + mt76_for_each_q_rx(mdev, i) {
> > + napi_enable(&mdev->napi[i]);
> > + napi_schedule(&mdev->napi[i]);
> > + }
> > + napi_enable(&mdev->tx_napi);
> > + napi_schedule(&mdev->tx_napi);
> > +
> > + return 0;
> > +}
> > +
> > MODULE_DEVICE_TABLE(pci, mt76pci_device_table);
> > MODULE_FIRMWARE(MT7662_FIRMWARE);
> > MODULE_FIRMWARE(MT7662_ROM_PATCH);
> > @@ -113,6 +167,10 @@ static struct pci_driver mt76pci_driver = {
> > .id_table = mt76pci_device_table,
> > .probe = mt76pci_probe,
> > .remove = mt76pci_remove,
> > +#ifdef CONFIG_PM
> > + .suspend= mt76x2e_suspend,
> > + .resume = mt76x2e_resume,
> > +#endif /* CONFIG_PM */
> > };
> >
> > module_pci_driver(mt76pci_driver);
> > --
> > 2.26.2
>
> Unfortunately, it seems it did little change to my setup. The resume
> time shrunk it seems (but is still noticeable), and the system survived
> 2 suspend/resume cycles, but after the third resume the following
> happened:
>
> ==
