* Re: [PATCH net-next v6 08/13] net: wwan: t7xx: Add data path interface
From: Ilpo Järvinen @ 2022-04-21 10:54 UTC (permalink / raw)
To: Ricardo Martinez
Cc: netdev, linux-wireless, kuba, davem, johannes, ryazanov.s.a,
loic.poulain, m.chetan.kumar, chandrashekar.devegowda, linuxwwan,
chiranjeevi.rapolu, haijun.liu, amir.hanania, andriy.shevchenko,
dinesh.sharma, eliot.lee, ilpo.johannes.jarvinen, moises.veleta,
pierre-louis.bossart, muralidharan.sethuraman,
Soumya.Prakash.Mishra, sreehari.kancharla, madhusmita.sahu
In-Reply-To: <20220407223629.21487-9-ricardo.martinez@linux.intel.com>
On Thu, 7 Apr 2022, Ricardo Martinez wrote:
> From: Haijun Liu <haijun.liu@mediatek.com>
>
> Data Path Modem AP Interface (DPMAIF) HIF layer provides methods
> for initialization, ISR, control and event handling of TX/RX flows.
>
> DPMAIF TX
> Exposes the 'dmpaif_tx_send_skb' function which can be used by the
> network device to transmit packets.
> The uplink data management uses a Descriptor Ring Buffer (DRB).
> First DRB entry is a message type that will be followed by 1 or more
> normal DRB entries. Message type DRB will hold the skb information
> and each normal DRB entry holds a pointer to the skb payload.
>
> DPMAIF RX
> The downlink buffer management uses Buffer Address Table (BAT) and
> Packet Information Table (PIT) rings.
> The BAT ring holds the address of skb data buffer for the HW to use,
> while the PIT contains metadata about a whole network packet including
> a reference to the BAT entry holding the data buffer address.
> The driver reads the PIT and BAT entries written by the modem, when
> reaching a threshold, the driver will reload the PIT and BAT rings.
>
> Signed-off-by: Haijun Liu <haijun.liu@mediatek.com>
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Co-developed-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
> Signed-off-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
>
> >From a WWAN framework perspective:
> Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
> + if (txq->tx_skb_head.qlen >= txq->tx_list_max_len)
> + goto report_full_state;
> +
> + skb_cb = T7XX_SKB_CB(skb);
> + skb_cb->txq_number = txq_number;
> + skb_queue_tail(&txq->tx_skb_head, skb);
> + wake_up(&dpmaif_ctrl->tx_wq);
> + return 0;
> +
> +report_full_state:
> + callbacks = dpmaif_ctrl->callbacks;
> + callbacks->state_notify(dpmaif_ctrl->t7xx_dev, DMPAIF_TXQ_STATE_FULL, txq_number>
> + return -EBUSY;
> +}
Should this actually report full earlier so that the queue can be stopped
before NETDEV_TX_BUSY has to be returned (by the callers in 09/13)?
(see Documentation/networking/driver.rst)
> + /* Wait for active Tx to be doneg */
doneg -> done
> +struct dpmaif_drb {
> + __le32 header;
> + union {
> + struct {
> + __le32 data_addr_l;
> + __le32 data_addr_h;
> + __le32 reserved;
> + } pd;
> + struct {
> + __le32 msg_hdr;
> + __le32 reserved1;
> + __le32 reserved2;
> + } msg;
> + };
> +};
"reserved" and "reserved2" could be placed outside of the union
as they have the same offset.
--
i.
^ permalink raw reply
* Re: [PATCH net-next v6 09/13] net: wwan: t7xx: Add WWAN network interface
From: Ilpo Järvinen @ 2022-04-21 10:56 UTC (permalink / raw)
To: Ricardo Martinez
Cc: netdev, linux-wireless, kuba, davem, johannes, ryazanov.s.a,
loic.poulain, m.chetan.kumar, chandrashekar.devegowda, linuxwwan,
chiranjeevi.rapolu, haijun.liu, amir.hanania, andriy.shevchenko,
dinesh.sharma, eliot.lee, ilpo.johannes.jarvinen, moises.veleta,
pierre-louis.bossart, muralidharan.sethuraman,
Soumya.Prakash.Mishra, sreehari.kancharla, madhusmita.sahu
In-Reply-To: <20220407223629.21487-10-ricardo.martinez@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 920 bytes --]
On Thu, 7 Apr 2022, Ricardo Martinez wrote:
> From: Haijun Liu <haijun.liu@mediatek.com>
>
> Creates the Cross Core Modem Network Interface (CCMNI) which implements
> the wwan_ops for registration with the WWAN framework, CCMNI also
> implements the net_device_ops functions used by the network device.
> Network device operations include open, close, start transmission, TX
> timeout and change MTU.
>
> Signed-off-by: Haijun Liu <haijun.liu@mediatek.com>
> Co-developed-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Co-developed-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
> Signed-off-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
>
> >From a WWAN framework perspective:
> Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply
* Re: [PATCH net] net: sched: act_mirred: Reset ct info when mirror/redirect skb
From: Hangbin Liu @ 2022-04-21 11:00 UTC (permalink / raw)
To: Eyal Birger
Cc: Marcelo Ricardo Leitner, netdev, jhs, xiyou.wangcong, jiri, davem,
kuba, ahleihel, dcaratti, aconole, roid, Shmulik Ladkani
In-Reply-To: <CAHsH6GvoDr5qOKsvvuShfHFi4CsCfaC-pUbxTE6OfYWhgTf9bg@mail.gmail.com>
Hi Eyal,
On Tue, Apr 19, 2022 at 09:14:38PM +0300, Eyal Birger wrote:
> > > > On Mon, 9 Aug 2021 15:04:55 +0800 you wrote:
> > > > > When mirror/redirect a skb to a different port, the ct info should be reset
> > > > > for reclassification. Or the pkts will match unexpected rules. For example,
> > > > > with following topology and commands:
> > > > >
> > > > > -----------
> > > > > |
> > > > > veth0 -+-------
> > > > > |
> > > > > veth1 -+-------
> > > > > |
> > > > >
> > > > > [...]
> > > >
> > > > Here is the summary with links:
> > > > - [net] net: sched: act_mirred: Reset ct info when mirror/redirect skb
> > > > https://git.kernel.org/netdev/net/c/d09c548dbf3b
> > >
> > > Unfortunately this commit breaks DNAT when performed before going via mirred
> > > egress->ingress.
> > >
> > > The reason is that connection tracking is lost and therefore a new state
> > > is created on ingress.
> > >
> > > This breaks existing setups.
> > >
> > > See below a simplified script reproducing this issue.
I think we come in to a paradox state. Some user don't want to have previous
ct info after mirror, while others would like to keep. In my understanding,
when we receive a pkt from a interface, the skb should be clean and no ct info
at first. But I may wrong.
Jamal, Wang Cong, Jiri, do you have any comments?
> >
> > I guess I can understand why the reproducer triggers it, but I fail to
> > see the actual use case you have behind it. Can you please elaborate
> > on it?
>
> One use case we use mirred egress->ingress redirect for is when we want to
> reroute a packet after applying some change to the packet which would affect
> its routing. for example consider a bpf program running on tc ingress (after
> mirred) setting the skb->mark based on some criteria.
>
> So you have something like:
>
> packet routed to dummy device based on some criteria ->
> mirred redirect to ingress ->
> classification by ebpf logic at tc ingress ->
> packet routed again
>
> We have a setup where DNAT is performed before this flow in that case the
> ebpf logic needs to see the packet after the NAT.
Is it possible to check whether it's need to set the skb->mark before DNAT?
So we can update it before egress and no need to re-route.
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH net-next v6 10/13] net: wwan: t7xx: Introduce power management
From: Ilpo Järvinen @ 2022-04-21 11:01 UTC (permalink / raw)
To: Ricardo Martinez
Cc: Netdev, linux-wireless, kuba, davem, johannes, ryazanov.s.a,
loic.poulain, m.chetan.kumar, chandrashekar.devegowda, linuxwwan,
chiranjeevi.rapolu, haijun.liu, amir.hanania, Andy Shevchenko,
dinesh.sharma, eliot.lee, moises.veleta, pierre-louis.bossart,
muralidharan.sethuraman, Soumya.Prakash.Mishra,
sreehari.kancharla, madhusmita.sahu
In-Reply-To: <20220407223629.21487-11-ricardo.martinez@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 983 bytes --]
On Thu, 7 Apr 2022, Ricardo Martinez wrote:
> From: Haijun Liu <haijun.liu@mediatek.com>
>
> Implements suspend, resumes, freeze, thaw, poweroff, and restore
> `dev_pm_ops` callbacks.
>
> >From the host point of view, the t7xx driver is one entity. But, the
> device has several modules that need to be addressed in different ways
> during power management (PM) flows.
> The driver uses the term 'PM entities' to refer to the 2 DPMA and
> 2 CLDMA HW blocks that need to be managed during PM flows.
> When a dev_pm_ops function is called, the PM entities list is iterated
> and the matching function is called for each entry in the list.
>
> Signed-off-by: Haijun Liu <haijun.liu@mediatek.com>
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Co-developed-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
> Signed-off-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply
* Re: [PATCH net-next v6 11/13] net: wwan: t7xx: Runtime PM
From: Ilpo Järvinen @ 2022-04-21 11:03 UTC (permalink / raw)
To: Ricardo Martinez
Cc: Netdev, linux-wireless, kuba, davem, johannes, ryazanov.s.a,
loic.poulain, m.chetan.kumar, chandrashekar.devegowda, linuxwwan,
chiranjeevi.rapolu, haijun.liu, amir.hanania, Andy Shevchenko,
dinesh.sharma, eliot.lee, moises.veleta, pierre-louis.bossart,
muralidharan.sethuraman, Soumya.Prakash.Mishra,
sreehari.kancharla, madhusmita.sahu
In-Reply-To: <20220407223629.21487-12-ricardo.martinez@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 622 bytes --]
On Thu, 7 Apr 2022, Ricardo Martinez wrote:
> From: Haijun Liu <haijun.liu@mediatek.com>
>
> Enables runtime power management callbacks including runtime_suspend
> and runtime_resume. Autosuspend is used to prevent overhead by frequent
> wake-ups.
>
> Signed-off-by: Haijun Liu <haijun.liu@mediatek.com>
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Co-developed-by: Eliot Lee <eliot.lee@intel.com>
> Signed-off-by: Eliot Lee <eliot.lee@intel.com>
> Signed-off-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply
* Re: [PATCH] vDPA/ifcvf: allow userspace to suspend a queue
From: Zhu, Lingshan @ 2022-04-21 11:05 UTC (permalink / raw)
To: Zhou Furong, jasowang, mst; +Cc: virtualization, netdev
In-Reply-To: <b24c4e3d-2792-da43-3335-5ed83c557565@linux.intel.com>
On 4/12/2022 2:55 PM, Zhou Furong wrote:
> Hi,
>
>> +bool ifcvf_get_vq_ready(struct ifcvf_hw *hw, u16 qid)
>> +{
>> + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>> + bool queue_enable;
>> +
>> + vp_iowrite16(qid, &cfg->queue_select);
>> + queue_enable = vp_ioread16(&cfg->queue_enable);
>> +
>> + return (bool)queue_enable;
> queue_enable is bool, why cast? looks like remove the variable is better.
> return vp_ioread16(&cfg->queue_enable);
Hi, thanks for your comments. This cast tries to make some static
checkers happy.
Please correct me if I misunderstood it:
vp_ioread16() returns an u16, and bool is not an one bit variable, it is
C99 _Bool.
C99 defines _Bool to be true(expends to int 1) or false(expends to int 0).
I think this implies a bool is actually capable to store an u16.
so I am afraid some checkers may complain about "queue_enable =
vp_ioread16(&cfg->queue_enable);",
a cast here can help us silence the checkers.
see http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf section 7.16
and https://www.kernel.org/doc/Documentation/process/coding-style.rst
section 17
Thanks,
Zhu Lingshan
>
>> static bool ifcvf_vdpa_get_vq_ready(struct vdpa_device *vdpa_dev,
>> u16 qid)
>> {
>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>> + bool ready;
>> - return vf->vring[qid].ready;
>> + ready = ifcvf_get_vq_ready(vf, qid);
>> +
>> + return ready;
> remove ready looks better
> return ifcvf_get_vq_ready(vf, qid);
>
>
> Best regards,
> Furong
^ permalink raw reply
* Re: [PATCH] vDPA/ifcvf: allow userspace to suspend a queue
From: Zhu, Lingshan @ 2022-04-21 11:11 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin
Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org
In-Reply-To: <CACGkMEu7dUYKr7Nv-fDFFBM4M1hvWuO8P17xNMEkwofiiP178A@mail.gmail.com>
On 4/13/2022 4:25 PM, Jason Wang wrote:
> On Mon, Apr 11, 2022 at 11:18 AM Zhu Lingshan<lingshan.zhu@intel.com> wrote:
>> Formerly, ifcvf driver has implemented a lazy-initialization mechanism
>> for the virtqueues, it would store all virtqueue config fields that
>> passed down from the userspace, then load them to the virtqueues and
>> enable the queues upon DRIVER_OK.
>>
>> To allow the userspace to suspend a virtqueue,
>> this commit passes queue_enable to the virtqueue directly through
>> set_vq_ready().
>>
>> This feature requires and this commits implementing all virtqueue
>> ops(set_vq_addr, set_vq_num and set_vq_ready) to take immediate
>> actions than lazy-initialization, so ifcvf_hw_enable() is retired.
>>
>> To avoid losing virtqueue configurations caused by multiple
>> rounds of reset(), this commit also refactors thed evice reset
>> routine, now it simply reset the config handler and the virtqueues,
>> and only once device-reset().
>>
>> Signed-off-by: Zhu Lingshan<lingshan.zhu@intel.com>
>> ---
>> drivers/vdpa/ifcvf/ifcvf_base.c | 94 ++++++++++++++++++++-------------
>> drivers/vdpa/ifcvf/ifcvf_base.h | 11 ++--
>> drivers/vdpa/ifcvf/ifcvf_main.c | 57 +++++---------------
>> 3 files changed, 75 insertions(+), 87 deletions(-)
>>
>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
>> index 48c4dadb0c7c..19eb0dcac123 100644
>> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
>> @@ -175,16 +175,12 @@ u8 ifcvf_get_status(struct ifcvf_hw *hw)
>> void ifcvf_set_status(struct ifcvf_hw *hw, u8 status)
>> {
>> vp_iowrite8(status, &hw->common_cfg->device_status);
>> + vp_ioread8(&hw->common_cfg->device_status);
> This looks confusing, the name of the function is to set the status
> but what actually implemented here is to get the status.
this read is to flush IO, however I think we don't need to flush the
IO requests until DRIVER_OK will make sure they flushed.
>
>> }
>>
>> void ifcvf_reset(struct ifcvf_hw *hw)
>> {
>> - hw->config_cb.callback = NULL;
>> - hw->config_cb.private = NULL;
>> -
>> ifcvf_set_status(hw, 0);
>> - /* flush set_status, make sure VF is stopped, reset */
>> - ifcvf_get_status(hw);
>> }
>>
>> static void ifcvf_add_status(struct ifcvf_hw *hw, u8 status)
>> @@ -331,68 +327,94 @@ int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num)
>> ifcvf_lm = (struct ifcvf_lm_cfg __iomem *)hw->lm_cfg;
>> q_pair_id = qid / hw->nr_vring;
>> avail_idx_addr = &ifcvf_lm->vring_lm_cfg[q_pair_id].idx_addr[qid % 2];
>> - hw->vring[qid].last_avail_idx = num;
>> vp_iowrite16(num, avail_idx_addr);
>> + vp_ioread16(avail_idx_addr);
> This looks like a bug fix.
same as above, this flush has been removed, write DRIVER_OK to flush
them once for all.
>
>> return 0;
>> }
>>
>> -static int ifcvf_hw_enable(struct ifcvf_hw *hw)
>> +void ifcvf_set_vq_num(struct ifcvf_hw *hw, u16 qid, u32 num)
>> {
>> - struct virtio_pci_common_cfg __iomem *cfg;
>> - u32 i;
>> + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>>
>> - cfg = hw->common_cfg;
>> - for (i = 0; i < hw->nr_vring; i++) {
>> - if (!hw->vring[i].ready)
>> - break;
>> + vp_iowrite16(qid, &cfg->queue_select);
>> + vp_iowrite16(num, &cfg->queue_size);
>> + vp_ioread16(&cfg->queue_size);
>> +}
>>
>> - vp_iowrite16(i, &cfg->queue_select);
>> - vp_iowrite64_twopart(hw->vring[i].desc, &cfg->queue_desc_lo,
>> - &cfg->queue_desc_hi);
>> - vp_iowrite64_twopart(hw->vring[i].avail, &cfg->queue_avail_lo,
>> - &cfg->queue_avail_hi);
>> - vp_iowrite64_twopart(hw->vring[i].used, &cfg->queue_used_lo,
>> - &cfg->queue_used_hi);
>> - vp_iowrite16(hw->vring[i].size, &cfg->queue_size);
>> - ifcvf_set_vq_state(hw, i, hw->vring[i].last_avail_idx);
>> - vp_iowrite16(1, &cfg->queue_enable);
>> - }
>> +int ifcvf_set_vq_address(struct ifcvf_hw *hw, u16 qid, u64 desc_area,
>> + u64 driver_area, u64 device_area)
>> +{
>> + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>> +
>> + vp_iowrite16(qid, &cfg->queue_select);
>> + vp_iowrite64_twopart(desc_area, &cfg->queue_desc_lo,
>> + &cfg->queue_desc_hi);
>> + vp_iowrite64_twopart(driver_area, &cfg->queue_avail_lo,
>> + &cfg->queue_avail_hi);
>> + vp_iowrite64_twopart(device_area, &cfg->queue_used_lo,
>> + &cfg->queue_used_hi);
>> + /* to flush IO */
>> + vp_ioread16(&cfg->queue_select);
> Why do we need to flush I/O here?
yes, same as above
>> return 0;
>> }
>>
>> -static void ifcvf_hw_disable(struct ifcvf_hw *hw)
>> +void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid, bool ready)
>> {
>> - u32 i;
>> + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>>
>> - ifcvf_set_config_vector(hw, VIRTIO_MSI_NO_VECTOR);
>> - for (i = 0; i < hw->nr_vring; i++) {
>> - ifcvf_set_vq_vector(hw, i, VIRTIO_MSI_NO_VECTOR);
>> - }
>> + vp_iowrite16(qid, &cfg->queue_select);
>> + vp_iowrite16(ready, &cfg->queue_enable);
> I think we need a comment to explain that IFCVF can support write to
> queue_enable since it's not allowed by the virtio spec.
sure
>
>> + vp_ioread16(&cfg->queue_enable);
>> +}
>> +
>> +bool ifcvf_get_vq_ready(struct ifcvf_hw *hw, u16 qid)
>> +{
>> + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>> + bool queue_enable;
>> +
>> + vp_iowrite16(qid, &cfg->queue_select);
>> + queue_enable = vp_ioread16(&cfg->queue_enable);
>> +
>> + return (bool)queue_enable;
>> }
>>
>> int ifcvf_start_hw(struct ifcvf_hw *hw)
>> {
>> - ifcvf_reset(hw);
>> ifcvf_add_status(hw, VIRTIO_CONFIG_S_ACKNOWLEDGE);
>> ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER);
>>
>> if (ifcvf_config_features(hw) < 0)
>> return -EINVAL;
>>
>> - if (ifcvf_hw_enable(hw) < 0)
>> - return -EINVAL;
>> -
>> ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER_OK);
>>
>> return 0;
>> }
>>
>> +static void ifcvf_reset_vring(struct ifcvf_hw *hw)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < hw->nr_vring; i++) {
>> + hw->vring[i].cb.callback = NULL;
>> + hw->vring[i].cb.private = NULL;
>> + ifcvf_set_vq_vector(hw, i, VIRTIO_MSI_NO_VECTOR);
>> + }
>> +}
>> +
>> +static void ifcvf_reset_config_handler(struct ifcvf_hw *hw)
>> +{
>> + hw->config_cb.callback = NULL;
>> + hw->config_cb.private = NULL;
>> + ifcvf_set_config_vector(hw, VIRTIO_MSI_NO_VECTOR);
> Do we need to synchronize with the IRQ here?
added synchronize_irq for both reset_vring() and reset_config_handler()
>> +}
>> +
>> void ifcvf_stop_hw(struct ifcvf_hw *hw)
>> {
>> - ifcvf_hw_disable(hw);
>> - ifcvf_reset(hw);
>> + ifcvf_reset_vring(hw);
>> + ifcvf_reset_config_handler(hw);
>> }
>>
>> void ifcvf_notify_queue(struct ifcvf_hw *hw, u16 qid)
>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
>> index 115b61f4924b..41d86985361f 100644
>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
>> @@ -49,12 +49,6 @@
>> #define MSIX_VECTOR_DEV_SHARED 3
>>
>> struct vring_info {
>> - u64 desc;
>> - u64 avail;
>> - u64 used;
>> - u16 size;
>> - u16 last_avail_idx;
>> - bool ready;
>> void __iomem *notify_addr;
>> phys_addr_t notify_pa;
>> u32 irq;
>> @@ -131,6 +125,11 @@ int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num);
>> struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw);
>> int ifcvf_probed_virtio_net(struct ifcvf_hw *hw);
>> u32 ifcvf_get_config_size(struct ifcvf_hw *hw);
>> +int ifcvf_set_vq_address(struct ifcvf_hw *hw, u16 qid, u64 desc_area,
>> + u64 driver_area, u64 device_area);
>> u16 ifcvf_set_vq_vector(struct ifcvf_hw *hw, u16 qid, int vector);
>> u16 ifcvf_set_config_vector(struct ifcvf_hw *hw, int vector);
>> +void ifcvf_set_vq_num(struct ifcvf_hw *hw, u16 qid, u32 num);
>> +void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid, bool ready);
>> +bool ifcvf_get_vq_ready(struct ifcvf_hw *hw, u16 qid);
>> #endif /* _IFCVF_H_ */
>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
>> index 4366320fb68d..e442aa11333e 100644
>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
>> @@ -374,37 +374,6 @@ static int ifcvf_start_datapath(void *private)
>> return ret;
>> }
>>
>> -static int ifcvf_stop_datapath(void *private)
>> -{
>> - struct ifcvf_hw *vf = ifcvf_private_to_vf(private);
>> - int i;
>> -
>> - for (i = 0; i < vf->nr_vring; i++)
>> - vf->vring[i].cb.callback = NULL;
>> -
>> - ifcvf_stop_hw(vf);
>> -
>> - return 0;
>> -}
>> -
>> -static void ifcvf_reset_vring(struct ifcvf_adapter *adapter)
>> -{
>> - struct ifcvf_hw *vf = ifcvf_private_to_vf(adapter);
>> - int i;
>> -
>> - for (i = 0; i < vf->nr_vring; i++) {
>> - vf->vring[i].last_avail_idx = 0;
>> - vf->vring[i].desc = 0;
>> - vf->vring[i].avail = 0;
>> - vf->vring[i].used = 0;
>> - vf->vring[i].ready = 0;
>> - vf->vring[i].cb.callback = NULL;
>> - vf->vring[i].cb.private = NULL;
>> - }
>> -
>> - ifcvf_reset(vf);
>> -}
>> -
>> static struct ifcvf_adapter *vdpa_to_adapter(struct vdpa_device *vdpa_dev)
>> {
>> return container_of(vdpa_dev, struct ifcvf_adapter, vdpa);
>> @@ -477,6 +446,8 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
>> if (status_old == status)
>> return;
>>
>> + ifcvf_set_status(vf, status);
>> +
>> if ((status & VIRTIO_CONFIG_S_DRIVER_OK) &&
>> !(status_old & VIRTIO_CONFIG_S_DRIVER_OK)) {
>> ret = ifcvf_request_irq(adapter);
> Does this mean e.g for DRIVER_OK the device may work before the
> interrupt handler is requested?
now we call ifcvf_set_status() which write status to the device in the
end of function
ifcvf_vdpa_set_status(), so it has a chance to check whether it's
setting DRIVER_OK,
then it can request_irq upon DRIVER_OK, no gaps anymore.
> Looks racy.
>
> Thanks
>
>> @@ -493,7 +464,6 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
>> status);
>> }
>>
>> - ifcvf_set_status(vf, status);
>> }
>>
>> static int ifcvf_vdpa_reset(struct vdpa_device *vdpa_dev)
>> @@ -509,12 +479,10 @@ static int ifcvf_vdpa_reset(struct vdpa_device *vdpa_dev)
>> if (status_old == 0)
>> return 0;
>>
>> - if (status_old & VIRTIO_CONFIG_S_DRIVER_OK) {
>> - ifcvf_stop_datapath(adapter);
>> - ifcvf_free_irq(adapter);
>> - }
>> + ifcvf_stop_hw(vf);
>> + ifcvf_free_irq(adapter);
>>
>> - ifcvf_reset_vring(adapter);
>> + ifcvf_reset(vf);
>>
>> return 0;
>> }
>> @@ -554,14 +522,17 @@ static void ifcvf_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev,
>> {
>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>
>> - vf->vring[qid].ready = ready;
>> + ifcvf_set_vq_ready(vf, qid, ready);
>> }
>>
>> static bool ifcvf_vdpa_get_vq_ready(struct vdpa_device *vdpa_dev, u16 qid)
>> {
>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>> + bool ready;
>>
>> - return vf->vring[qid].ready;
>> + ready = ifcvf_get_vq_ready(vf, qid);
>> +
>> + return ready;
>> }
>>
>> static void ifcvf_vdpa_set_vq_num(struct vdpa_device *vdpa_dev, u16 qid,
>> @@ -569,7 +540,7 @@ static void ifcvf_vdpa_set_vq_num(struct vdpa_device *vdpa_dev, u16 qid,
>> {
>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>
>> - vf->vring[qid].size = num;
>> + ifcvf_set_vq_num(vf, qid, num);
>> }
>>
>> static int ifcvf_vdpa_set_vq_address(struct vdpa_device *vdpa_dev, u16 qid,
>> @@ -578,11 +549,7 @@ static int ifcvf_vdpa_set_vq_address(struct vdpa_device *vdpa_dev, u16 qid,
>> {
>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>
>> - vf->vring[qid].desc = desc_area;
>> - vf->vring[qid].avail = driver_area;
>> - vf->vring[qid].used = device_area;
>> -
>> - return 0;
>> + return ifcvf_set_vq_address(vf, qid, desc_area, driver_area, device_area);
>> }
>>
>> static void ifcvf_vdpa_kick_vq(struct vdpa_device *vdpa_dev, u16 qid)
>> --
>> 2.31.1
>>
^ permalink raw reply
* Re: [PATCH] vDPA/ifcvf: allow userspace to suspend a queue
From: Zhu, Lingshan @ 2022-04-21 11:13 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang; +Cc: virtualization, netdev
In-Reply-To: <20220413045223-mutt-send-email-mst@kernel.org>
On 4/13/2022 4:54 PM, Michael S. Tsirkin wrote:
> On Wed, Apr 13, 2022 at 04:25:22PM +0800, Jason Wang wrote:
>> On Mon, Apr 11, 2022 at 11:18 AM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>>> Formerly, ifcvf driver has implemented a lazy-initialization mechanism
>>> for the virtqueues, it would store all virtqueue config fields that
>>> passed down from the userspace, then load them to the virtqueues and
>>> enable the queues upon DRIVER_OK.
>>>
>>> To allow the userspace to suspend a virtqueue,
>>> this commit passes queue_enable to the virtqueue directly through
>>> set_vq_ready().
>>>
>>> This feature requires and this commits implementing all virtqueue
>>> ops(set_vq_addr, set_vq_num and set_vq_ready) to take immediate
>>> actions than lazy-initialization, so ifcvf_hw_enable() is retired.
>>>
>>> To avoid losing virtqueue configurations caused by multiple
>>> rounds of reset(), this commit also refactors thed evice reset
>>> routine, now it simply reset the config handler and the virtqueues,
>>> and only once device-reset().
>>>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>> ---
>>> drivers/vdpa/ifcvf/ifcvf_base.c | 94 ++++++++++++++++++++-------------
>>> drivers/vdpa/ifcvf/ifcvf_base.h | 11 ++--
>>> drivers/vdpa/ifcvf/ifcvf_main.c | 57 +++++---------------
>>> 3 files changed, 75 insertions(+), 87 deletions(-)
>>>
>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
>>> index 48c4dadb0c7c..19eb0dcac123 100644
>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
>>> @@ -175,16 +175,12 @@ u8 ifcvf_get_status(struct ifcvf_hw *hw)
>>> void ifcvf_set_status(struct ifcvf_hw *hw, u8 status)
>>> {
>>> vp_iowrite8(status, &hw->common_cfg->device_status);
>>> + vp_ioread8(&hw->common_cfg->device_status);
>> This looks confusing, the name of the function is to set the status
>> but what actually implemented here is to get the status.
>>
>>> }
>>>
>>> void ifcvf_reset(struct ifcvf_hw *hw)
>>> {
>>> - hw->config_cb.callback = NULL;
>>> - hw->config_cb.private = NULL;
>>> -
>>> ifcvf_set_status(hw, 0);
>>> - /* flush set_status, make sure VF is stopped, reset */
>>> - ifcvf_get_status(hw);
>>> }
>>>
>>> static void ifcvf_add_status(struct ifcvf_hw *hw, u8 status)
>>> @@ -331,68 +327,94 @@ int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num)
>>> ifcvf_lm = (struct ifcvf_lm_cfg __iomem *)hw->lm_cfg;
>>> q_pair_id = qid / hw->nr_vring;
>>> avail_idx_addr = &ifcvf_lm->vring_lm_cfg[q_pair_id].idx_addr[qid % 2];
>>> - hw->vring[qid].last_avail_idx = num;
>>> vp_iowrite16(num, avail_idx_addr);
>>> + vp_ioread16(avail_idx_addr);
>> This looks like a bug fix.
> is this to flush out the status write? pls add a comment
> explaining when and why it's needed.
I think this may not needed anymore, setting DRIVER_OK will flush them all,
so all extra flush are removed.
>
>>> return 0;
>>> }
>>>
>>> -static int ifcvf_hw_enable(struct ifcvf_hw *hw)
>>> +void ifcvf_set_vq_num(struct ifcvf_hw *hw, u16 qid, u32 num)
>>> {
>>> - struct virtio_pci_common_cfg __iomem *cfg;
>>> - u32 i;
>>> + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>>>
>>> - cfg = hw->common_cfg;
>>> - for (i = 0; i < hw->nr_vring; i++) {
>>> - if (!hw->vring[i].ready)
>>> - break;
>>> + vp_iowrite16(qid, &cfg->queue_select);
>>> + vp_iowrite16(num, &cfg->queue_size);
>>> + vp_ioread16(&cfg->queue_size);
>>> +}
>>>
>>> - vp_iowrite16(i, &cfg->queue_select);
>>> - vp_iowrite64_twopart(hw->vring[i].desc, &cfg->queue_desc_lo,
>>> - &cfg->queue_desc_hi);
>>> - vp_iowrite64_twopart(hw->vring[i].avail, &cfg->queue_avail_lo,
>>> - &cfg->queue_avail_hi);
>>> - vp_iowrite64_twopart(hw->vring[i].used, &cfg->queue_used_lo,
>>> - &cfg->queue_used_hi);
>>> - vp_iowrite16(hw->vring[i].size, &cfg->queue_size);
>>> - ifcvf_set_vq_state(hw, i, hw->vring[i].last_avail_idx);
>>> - vp_iowrite16(1, &cfg->queue_enable);
>>> - }
>>> +int ifcvf_set_vq_address(struct ifcvf_hw *hw, u16 qid, u64 desc_area,
>>> + u64 driver_area, u64 device_area)
>>> +{
>>> + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>>> +
>>> + vp_iowrite16(qid, &cfg->queue_select);
>>> + vp_iowrite64_twopart(desc_area, &cfg->queue_desc_lo,
>>> + &cfg->queue_desc_hi);
>>> + vp_iowrite64_twopart(driver_area, &cfg->queue_avail_lo,
>>> + &cfg->queue_avail_hi);
>>> + vp_iowrite64_twopart(device_area, &cfg->queue_used_lo,
>>> + &cfg->queue_used_hi);
>>> + /* to flush IO */
>>> + vp_ioread16(&cfg->queue_select);
>> Why do we need to flush I/O here?
>>
>>> return 0;
>>> }
>>>
>>> -static void ifcvf_hw_disable(struct ifcvf_hw *hw)
>>> +void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid, bool ready)
>>> {
>>> - u32 i;
>>> + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>>>
>>> - ifcvf_set_config_vector(hw, VIRTIO_MSI_NO_VECTOR);
>>> - for (i = 0; i < hw->nr_vring; i++) {
>>> - ifcvf_set_vq_vector(hw, i, VIRTIO_MSI_NO_VECTOR);
>>> - }
>>> + vp_iowrite16(qid, &cfg->queue_select);
>>> + vp_iowrite16(ready, &cfg->queue_enable);
>> I think we need a comment to explain that IFCVF can support write to
>> queue_enable since it's not allowed by the virtio spec.
>
> I think you mean writing 0 there. writing 1 is allowed.
I have added a comment here says writing 0 would suspend the queue.
>
>
>>> + vp_ioread16(&cfg->queue_enable);
>>> +}
>>> +
>>> +bool ifcvf_get_vq_ready(struct ifcvf_hw *hw, u16 qid)
>>> +{
>>> + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>>> + bool queue_enable;
>>> +
>>> + vp_iowrite16(qid, &cfg->queue_select);
>>> + queue_enable = vp_ioread16(&cfg->queue_enable);
>>> +
>>> + return (bool)queue_enable;
>>> }
>>>
>>> int ifcvf_start_hw(struct ifcvf_hw *hw)
>>> {
>>> - ifcvf_reset(hw);
>>> ifcvf_add_status(hw, VIRTIO_CONFIG_S_ACKNOWLEDGE);
>>> ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER);
>>>
>>> if (ifcvf_config_features(hw) < 0)
>>> return -EINVAL;
>>>
>>> - if (ifcvf_hw_enable(hw) < 0)
>>> - return -EINVAL;
>>> -
>>> ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER_OK);
>>>
>>> return 0;
>>> }
>>>
>>> +static void ifcvf_reset_vring(struct ifcvf_hw *hw)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < hw->nr_vring; i++) {
>>> + hw->vring[i].cb.callback = NULL;
>>> + hw->vring[i].cb.private = NULL;
>>> + ifcvf_set_vq_vector(hw, i, VIRTIO_MSI_NO_VECTOR);
>>> + }
>>> +}
>>> +
>>> +static void ifcvf_reset_config_handler(struct ifcvf_hw *hw)
>>> +{
>>> + hw->config_cb.callback = NULL;
>>> + hw->config_cb.private = NULL;
>>> + ifcvf_set_config_vector(hw, VIRTIO_MSI_NO_VECTOR);
>> Do we need to synchronize with the IRQ here?
>>
>>> +}
>>> +
>>> void ifcvf_stop_hw(struct ifcvf_hw *hw)
>>> {
>>> - ifcvf_hw_disable(hw);
>>> - ifcvf_reset(hw);
>>> + ifcvf_reset_vring(hw);
>>> + ifcvf_reset_config_handler(hw);
>>> }
>>>
>>> void ifcvf_notify_queue(struct ifcvf_hw *hw, u16 qid)
>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
>>> index 115b61f4924b..41d86985361f 100644
>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
>>> @@ -49,12 +49,6 @@
>>> #define MSIX_VECTOR_DEV_SHARED 3
>>>
>>> struct vring_info {
>>> - u64 desc;
>>> - u64 avail;
>>> - u64 used;
>>> - u16 size;
>>> - u16 last_avail_idx;
>>> - bool ready;
>>> void __iomem *notify_addr;
>>> phys_addr_t notify_pa;
>>> u32 irq;
>>> @@ -131,6 +125,11 @@ int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num);
>>> struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw);
>>> int ifcvf_probed_virtio_net(struct ifcvf_hw *hw);
>>> u32 ifcvf_get_config_size(struct ifcvf_hw *hw);
>>> +int ifcvf_set_vq_address(struct ifcvf_hw *hw, u16 qid, u64 desc_area,
>>> + u64 driver_area, u64 device_area);
>>> u16 ifcvf_set_vq_vector(struct ifcvf_hw *hw, u16 qid, int vector);
>>> u16 ifcvf_set_config_vector(struct ifcvf_hw *hw, int vector);
>>> +void ifcvf_set_vq_num(struct ifcvf_hw *hw, u16 qid, u32 num);
>>> +void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid, bool ready);
>>> +bool ifcvf_get_vq_ready(struct ifcvf_hw *hw, u16 qid);
>>> #endif /* _IFCVF_H_ */
>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
>>> index 4366320fb68d..e442aa11333e 100644
>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
>>> @@ -374,37 +374,6 @@ static int ifcvf_start_datapath(void *private)
>>> return ret;
>>> }
>>>
>>> -static int ifcvf_stop_datapath(void *private)
>>> -{
>>> - struct ifcvf_hw *vf = ifcvf_private_to_vf(private);
>>> - int i;
>>> -
>>> - for (i = 0; i < vf->nr_vring; i++)
>>> - vf->vring[i].cb.callback = NULL;
>>> -
>>> - ifcvf_stop_hw(vf);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -static void ifcvf_reset_vring(struct ifcvf_adapter *adapter)
>>> -{
>>> - struct ifcvf_hw *vf = ifcvf_private_to_vf(adapter);
>>> - int i;
>>> -
>>> - for (i = 0; i < vf->nr_vring; i++) {
>>> - vf->vring[i].last_avail_idx = 0;
>>> - vf->vring[i].desc = 0;
>>> - vf->vring[i].avail = 0;
>>> - vf->vring[i].used = 0;
>>> - vf->vring[i].ready = 0;
>>> - vf->vring[i].cb.callback = NULL;
>>> - vf->vring[i].cb.private = NULL;
>>> - }
>>> -
>>> - ifcvf_reset(vf);
>>> -}
>>> -
>>> static struct ifcvf_adapter *vdpa_to_adapter(struct vdpa_device *vdpa_dev)
>>> {
>>> return container_of(vdpa_dev, struct ifcvf_adapter, vdpa);
>>> @@ -477,6 +446,8 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
>>> if (status_old == status)
>>> return;
>>>
>>> + ifcvf_set_status(vf, status);
>>> +
>>> if ((status & VIRTIO_CONFIG_S_DRIVER_OK) &&
>>> !(status_old & VIRTIO_CONFIG_S_DRIVER_OK)) {
>>> ret = ifcvf_request_irq(adapter);
>> Does this mean e.g for DRIVER_OK the device may work before the
>> interrupt handler is requested?
>>
>> Looks racy.
>>
>> Thanks
>>
>>> @@ -493,7 +464,6 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
>>> status);
>>> }
>>>
>>> - ifcvf_set_status(vf, status);
>>> }
>>>
>>> static int ifcvf_vdpa_reset(struct vdpa_device *vdpa_dev)
>>> @@ -509,12 +479,10 @@ static int ifcvf_vdpa_reset(struct vdpa_device *vdpa_dev)
>>> if (status_old == 0)
>>> return 0;
>>>
>>> - if (status_old & VIRTIO_CONFIG_S_DRIVER_OK) {
>>> - ifcvf_stop_datapath(adapter);
>>> - ifcvf_free_irq(adapter);
>>> - }
>>> + ifcvf_stop_hw(vf);
>>> + ifcvf_free_irq(adapter);
>>>
>>> - ifcvf_reset_vring(adapter);
>>> + ifcvf_reset(vf);
>>>
>>> return 0;
>>> }
>>> @@ -554,14 +522,17 @@ static void ifcvf_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev,
>>> {
>>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>>
>>> - vf->vring[qid].ready = ready;
>>> + ifcvf_set_vq_ready(vf, qid, ready);
>>> }
>>>
>>> static bool ifcvf_vdpa_get_vq_ready(struct vdpa_device *vdpa_dev, u16 qid)
>>> {
>>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>> + bool ready;
>>>
>>> - return vf->vring[qid].ready;
>>> + ready = ifcvf_get_vq_ready(vf, qid);
>>> +
>>> + return ready;
>>> }
>>>
>>> static void ifcvf_vdpa_set_vq_num(struct vdpa_device *vdpa_dev, u16 qid,
>>> @@ -569,7 +540,7 @@ static void ifcvf_vdpa_set_vq_num(struct vdpa_device *vdpa_dev, u16 qid,
>>> {
>>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>>
>>> - vf->vring[qid].size = num;
>>> + ifcvf_set_vq_num(vf, qid, num);
>>> }
>>>
>>> static int ifcvf_vdpa_set_vq_address(struct vdpa_device *vdpa_dev, u16 qid,
>>> @@ -578,11 +549,7 @@ static int ifcvf_vdpa_set_vq_address(struct vdpa_device *vdpa_dev, u16 qid,
>>> {
>>> struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>>
>>> - vf->vring[qid].desc = desc_area;
>>> - vf->vring[qid].avail = driver_area;
>>> - vf->vring[qid].used = device_area;
>>> -
>>> - return 0;
>>> + return ifcvf_set_vq_address(vf, qid, desc_area, driver_area, device_area);
>>> }
>>>
>>> static void ifcvf_vdpa_kick_vq(struct vdpa_device *vdpa_dev, u16 qid)
>>> --
>>> 2.31.1
>>>
^ permalink raw reply
* Re: [PATCH] driver: usb: nullify dangling pointer in cdc_ncm_free
From: Oliver Neukum @ 2022-04-21 11:18 UTC (permalink / raw)
To: Andy Shevchenko, Johan Hovold, Oleksij Rempel
Cc: Dongliang Mu, Oliver Neukum, David S. Miller, Jakub Kicinski,
Paolo Abeni, Dongliang Mu, syzbot+eabbf2aaa999cc507108, USB,
netdev, Linux Kernel Mailing List, steve.glendinning
In-Reply-To: <CAHp75VeTqmdLhavZ+VbBYSFMDHr0FG4iKFGdbzE-wo5MCNikAA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 348 bytes --]
So I am afraid I have to ask again, whether anybody sees a fundamental
issue with the new version attached patch, as opposed to it not being an
elegant
solution?
I corrected the stuff Johan found and split the method in the asix driver.
I do not understand smsc95xx well, so I left it in the current state.
Regards
Oliver
[-- Attachment #2: 0001-usbnet-split-unbind-callback.patch --]
[-- Type: text/x-patch, Size: 4554 bytes --]
From 5953b3b12dd6cdd8d0bdb0119ee627d62219ab1e Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Thu, 10 Mar 2022 13:18:38 +0100
Subject: [PATCH] usbnet: split unbind callback
Some devices need to be informed of a disconnect before
the generic layer is informed, others need their notification
later to avoid race conditions. Hence we provide two callbacks.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/net/usb/asix_devices.c | 13 +++++++++++--
drivers/net/usb/smsc95xx.c | 4 ++--
drivers/net/usb/usbnet.c | 10 +++++++---
include/linux/usb/usbnet.h | 3 +++
4 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 38e47a93fb83..cde4d234b975 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -804,12 +804,18 @@ static int ax88772_stop(struct usbnet *dev)
return 0;
}
-static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
+static void ax88772_disable(struct usbnet *dev, struct usb_interface *intf)
{
struct asix_common_private *priv = dev->driver_priv;
phy_disconnect(priv->phydev);
- asix_rx_fixup_common_free(dev->driver_priv);
+}
+
+static void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
+{
+ struct asix_common_private *priv = dev->driver_priv;
+
+ asix_rx_fixup_common_free(priv);
}
static void ax88178_unbind(struct usbnet *dev, struct usb_interface *intf)
@@ -1211,6 +1217,7 @@ static const struct driver_info ax88772_info = {
.description = "ASIX AX88772 USB 2.0 Ethernet",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
+ .disable = ax88772_disable,
.status = asix_status,
.reset = ax88772_reset,
.stop = ax88772_stop,
@@ -1223,6 +1230,7 @@ static const struct driver_info ax88772b_info = {
.description = "ASIX AX88772B USB 2.0 Ethernet",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
+ .disable = ax88772_disable,
.status = asix_status,
.reset = ax88772_reset,
.stop = ax88772_stop,
@@ -1259,6 +1267,7 @@ static const struct driver_info hg20f9_info = {
.description = "HG20F9 USB 2.0 Ethernet",
.bind = ax88772_bind,
.unbind = ax88772_unbind,
+ .disable = ax88772_disable,
.status = asix_status,
.reset = ax88772_reset,
.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 4ef61f6b85df..20ce88373809 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1223,7 +1223,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
return ret;
}
-static void smsc95xx_unbind(struct usbnet *dev, struct usb_interface *intf)
+static void smsc95xx_disable(struct usbnet *dev, struct usb_interface *intf)
{
struct smsc95xx_priv *pdata = dev->driver_priv;
@@ -1997,7 +1997,7 @@ static int smsc95xx_manage_power(struct usbnet *dev, int on)
static const struct driver_info smsc95xx_info = {
.description = "smsc95xx USB 2.0 Ethernet",
.bind = smsc95xx_bind,
- .unbind = smsc95xx_unbind,
+ .disable = smsc95xx_disable,
.link_reset = smsc95xx_link_reset,
.reset = smsc95xx_reset,
.check_connect = smsc95xx_start_phy,
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index b1f93810a6f3..5f3851e61573 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1641,17 +1641,21 @@ void usbnet_disconnect (struct usb_interface *intf)
xdev->bus->bus_name, xdev->devpath,
dev->driver_info->description);
- if (dev->driver_info->unbind)
- dev->driver_info->unbind(dev, intf);
+ if (dev->driver_info->disable)
+ dev->driver_info->disable(dev, intf);
net = dev->net;
unregister_netdev (net);
+ usb_kill_urb(dev->interrupt);
+
cancel_work_sync(&dev->kevent);
usb_scuttle_anchored_urbs(&dev->deferred);
- usb_kill_urb(dev->interrupt);
+ if (dev->driver_info->unbind)
+ dev->driver_info->unbind (dev, intf);
+
usb_free_urb(dev->interrupt);
kfree(dev->padding_pkt);
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 1b4d72d5e891..dd4a1104e332 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -129,6 +129,9 @@ struct driver_info {
/* cleanup device ... can sleep, but can't fail */
void (*unbind)(struct usbnet *, struct usb_interface *);
+ /* disable device ... can sleep, but can't fail */
+ void (*disable)(struct usbnet *, struct usb_interface *);
+
/* reset device ... can sleep */
int (*reset)(struct usbnet *);
--
2.34.1
^ permalink raw reply related
* Re: 9p EBADF with cache enabled (Was: 9p fs-cache tests/benchmark (was: 9p fscache Duplicate cookie detected))
From: Christian Schoenebeck @ 2022-04-21 11:36 UTC (permalink / raw)
To: asmadeus, David Howells
Cc: dhowells, David Kahurani, davem, ericvh, kuba, linux-kernel,
lucho, netdev, v9fs-developer, Greg Kurz
In-Reply-To: <1050016.1650537372@warthog.procyon.org.uk>
On Donnerstag, 21. April 2022 12:36:12 CEST David Howells wrote:
> asmadeus@codewreck.org wrote:
> > int fd = open(argv[1], O_WRONLY|O_APPEND);
> > if (fd < 0)
> >
> > return 1;
> >
> > if (write(fd, "test\n", 5) < 0)
>
> I think I need to implement the ability to store writes in non-uptodate
> pages without needing to read from the server as NFS does. This may fix
> the performance drop also.
>
> David
I hope this does not sound harsh, wouldn't it make sense to revert
eb497943fa215897f2f60fd28aa6fe52da27ca6c for now until those issues are sorted
out? My concern is that it might take a long time to address them, and these
are not minor issues.
Best regards,
Christian Schoenebeck
^ permalink raw reply
* Re: [PATCH 1/5] net: mdio: Mask PHY only when its ACPI node is present
From: Andrew Lunn @ 2022-04-21 11:40 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: hkallweit1, linux, peppe.cavallaro, alexandre.torgue, joabreu,
davem, kuba, pabeni, netdev, linux-kernel
In-Reply-To: <CAAd53p5Wwn+HOMm1Z0VWcR_WrTzRvAGZOYg4X_txugSFd+EsDQ@mail.gmail.com>
On Thu, Apr 21, 2022 at 10:58:40AM +0800, Kai-Heng Feng wrote:
> On Wed, Apr 20, 2022 at 10:47 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Wed, Apr 20, 2022 at 08:40:48PM +0800, Kai-Heng Feng wrote:
> > > Not all PHY has an ACPI node, for those nodes auto probing is still
> > > needed.
> >
> > Why do you need this?
> >
> > Documentation/firmware-guide/acpi/dsd/phy.rst
> >
> > There is nothing here about there being PHYs which are not listed in
> > ACPI. If you have decided to go the ACPI route, you need to list the
> > PHYs.
>
> This is for backward-compatibility. MAC can have ACPI node but PHY may
> not have one.
And if the PHY does not have an ACPI node, fall back to
mdiobus_register(). This is what of_mdiobus_register() does. If
np=NULL, it calls mdiobus_register() and skips all the OF stuff.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v6 12/13] net: wwan: t7xx: Device deep sleep lock/unlock
From: Ilpo Järvinen @ 2022-04-21 11:47 UTC (permalink / raw)
To: Ricardo Martinez
Cc: Netdev, linux-wireless, kuba, davem, johannes, ryazanov.s.a,
loic.poulain, m.chetan.kumar, chandrashekar.devegowda, linuxwwan,
chiranjeevi.rapolu, haijun.liu, amir.hanania, Andy Shevchenko,
dinesh.sharma, eliot.lee, moises.veleta, pierre-louis.bossart,
muralidharan.sethuraman, Soumya.Prakash.Mishra,
sreehari.kancharla, madhusmita.sahu
In-Reply-To: <20220407223629.21487-13-ricardo.martinez@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]
On Thu, 7 Apr 2022, Ricardo Martinez wrote:
> From: Haijun Liu <haijun.liu@mediatek.com>
>
> Introduce the mechanism to lock/unlock the device 'deep sleep' mode.
> When the PCIe link state is L1.2 or L2, the host side still can keep
> the device is in D0 state from the host side point of view. At the same
> time, if the device's 'deep sleep' mode is unlocked, the device will
> go to 'deep sleep' while it is still in D0 state on the host side.
>
> Signed-off-by: Haijun Liu <haijun.liu@mediatek.com>
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Co-developed-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
> Signed-off-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
> ---
> +void t7xx_pci_enable_sleep(struct t7xx_pci_dev *t7xx_dev)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&t7xx_dev->md_pm_lock, flags);
> + t7xx_dev->sleep_disable_count--;
> + if (atomic_read(&t7xx_dev->md_pm_state) < MTK_PM_RESUMED) {
goto unlock;
> + spin_unlock_irqrestore(&t7xx_dev->md_pm_lock, flags);
> + return;
> + }
> +
> + if (t7xx_dev->sleep_disable_count == 0)
> + t7xx_dev_set_sleep_capability(t7xx_dev, true);
unlock:
> + spin_unlock_irqrestore(&t7xx_dev->md_pm_lock, flags);
> +}
> +
> static int t7xx_send_pm_request(struct t7xx_pci_dev *t7xx_dev, u32 request)
> {
> unsigned long wait_ret;
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply
* Re: [PATCH 4/5] net: phy: marvell: Add LED accessors for Marvell 88E1510
From: Andrew Lunn @ 2022-04-21 11:51 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: hkallweit1, linux, peppe.cavallaro, alexandre.torgue, joabreu,
davem, kuba, pabeni, netdev, linux-kernel
In-Reply-To: <CAAd53p6UAhDC2mGkz3_HgVs7kFgCwjfu2R+9FfROhToH2R6CjA@mail.gmail.com>
> This is not feasible.
> If BIOS can define a method and restore the LED by itself, it can put
> the method inside its S3 method and I don't have to work on this at
> the first place.
So maybe just declare the BIOS as FUBAR and move on to the next issue
assigned to you.
Do we really want the maintenance burden of this code for one machines
BIOS? Maybe the better solution is to push back on the vendor and its
BIOS, tell them how they should of done this, if the BIOS wants to be
in control of the LEDs it needs to offer the methods to control the
LEDs. And then hopefully the next machine the vendor produces will
have working BIOS.
Your other option is to take part in the effort to add control of the
LEDs via the standard Linux LED subsystem. The Marvel PHY driver is
likely to be one of the first to gain support this for. So you can
then totally take control of the LED from the BIOS and put it in the
users hands. And such a solution will be applicable to many machines,
not just one.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v6 02/13] net: wwan: t7xx: Add control DMA interface
From: Ilpo Järvinen @ 2022-04-21 11:55 UTC (permalink / raw)
To: Ricardo Martinez
Cc: Netdev, linux-wireless, kuba, davem, johannes, ryazanov.s.a,
loic.poulain, m.chetan.kumar, chandrashekar.devegowda, linuxwwan,
chiranjeevi.rapolu, haijun.liu, amir.hanania, Andy Shevchenko,
dinesh.sharma, eliot.lee, moises.veleta, pierre-louis.bossart,
muralidharan.sethuraman, Soumya.Prakash.Mishra,
sreehari.kancharla, madhusmita.sahu
In-Reply-To: <20220407223629.21487-3-ricardo.martinez@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 2352 bytes --]
On Thu, 7 Apr 2022, Ricardo Martinez wrote:
> From: Haijun Liu <haijun.liu@mediatek.com>
>
> Cross Layer DMA (CLDMA) Hardware interface (HIF) enables the control
> path of Host-Modem data transfers. CLDMA HIF layer provides a common
> interface to the Port Layer.
>
> CLDMA manages 8 independent RX/TX physical channels with data flow
> control in HW queues. CLDMA uses ring buffers of General Packet
> Descriptors (GPD) for TX/RX. GPDs can represent multiple or single
> data buffers (DB).
>
> CLDMA HIF initializes GPD rings, registers ISR handlers for CLDMA
> interrupts, and initializes CLDMA HW registers.
>
> CLDMA TX flow:
> 1. Port Layer write
> 2. Get DB address
> 3. Configure GPD
> 4. Triggering processing via HW register write
>
> CLDMA RX flow:
> 1. CLDMA HW sends a RX "done" to host
> 2. Driver starts thread to safely read GPD
> 3. DB is sent to Port layer
> 4. Create a new buffer for GPD ring
>
> Note: This patch does not enable compilation since it has dependencies
> such as t7xx_pcie_mac_clear_int()/t7xx_pcie_mac_set_int() and
> struct t7xx_pci_dev which are added by the core patch.
>
> Signed-off-by: Haijun Liu <haijun.liu@mediatek.com>
> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
> Co-developed-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
> Signed-off-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
>
> >From a WWAN framework perspective:
> Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> +struct cldma_tgpd {
> + u8 gpd_flags;
> + u8 not_used1;
> + u8 not_used2;
> + u8 debug_id;
> + __le32 next_gpd_ptr_h;
> + __le32 next_gpd_ptr_l;
> + __le32 data_buff_bd_ptr_h;
> + __le32 data_buff_bd_ptr_l;
> + __le16 data_buff_len;
> + __le16 not_used3;
> +};
> +
> +struct cldma_rgpd {
> + u8 gpd_flags;
> + u8 not_used1;
> + __le16 data_allow_len;
> + __le32 next_gpd_ptr_h;
> + __le32 next_gpd_ptr_l;
> + __le32 data_buff_bd_ptr_h;
> + __le32 data_buff_bd_ptr_l;
> + __le16 data_buff_len;
> + u8 not_used2;
> + u8 debug_id;
> +};
A small additional thing...
If you put next_gpd_ptr_h, next_gpd_ptr_l, data_buff_bd_ptr_h, and
data_buff_bd_ptr_l into another struct inside these structs,
t7xx_cldma_tgpd_set_data_ptr() and friends don't require duplicated
versions for tgpd and rgpd.
--
i.
^ permalink raw reply
* Re: [PATCH net-next v6 13/13] net: wwan: t7xx: Add maintainers and documentation
From: Ilpo Järvinen @ 2022-04-21 12:01 UTC (permalink / raw)
To: Ricardo Martinez
Cc: Netdev, linux-wireless, kuba, davem, johannes, ryazanov.s.a,
loic.poulain, m.chetan.kumar, chandrashekar.devegowda, linuxwwan,
chiranjeevi.rapolu, haijun.liu, amir.hanania, Andy Shevchenko,
dinesh.sharma, eliot.lee, moises.veleta, pierre-louis.bossart,
muralidharan.sethuraman, Soumya.Prakash.Mishra,
sreehari.kancharla, madhusmita.sahu
In-Reply-To: <20220407223629.21487-14-ricardo.martinez@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 368 bytes --]
On Thu, 7 Apr 2022, Ricardo Martinez wrote:
> Adds maintainers and documentation for MediaTek t7xx 5G WWAN modem
> device driver.
>
> Signed-off-by: Ricardo Martinez <ricardo.martinez@linux.intel.com>
>
> >From a WWAN framework perspective:
> Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply
* Re: [PATCH net] net/smc: sync err code when tcp connection was refused
From: liuyacan @ 2022-04-21 11:58 UTC (permalink / raw)
To: liuyacan
Cc: davem, kgraul, kuba, linux-kernel, linux-s390, netdev, pabeni,
tonylu, ubraun
In-Reply-To: <20220421094027.683992-1-liuyacan@corp.netease.com>
From: Yacan Liu <liuyacan@corp.netease.com>
Forgot to cc ubraun@linux.ibm.com
^ permalink raw reply
* [PATCH bpf-next] samples/bpf: detach xdp prog when program exits unexpectedly in xdp_rxq_info_user
From: Zhengchao Shao @ 2022-04-21 12:09 UTC (permalink / raw)
To: bpf, netdev, linux-kernel, ast, daniel, davem, kuba, hawk,
john.fastabend, andrii, kafai, songliubraving, yhs, kpsingh
Cc: weiyongjun1, shaozhengchao, yuehaibing
When xdp_rxq_info_user program exits unexpectedly, it doesn't detach xdp
prog of device, and other xdp prog can't be attached to the device. So
call init_exit() to detach xdp prog when program exits unexpectedly.
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
samples/bpf/xdp_rxq_info_user.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index f2d90cba5164..6378007e085a 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -18,7 +18,7 @@ static const char *__doc__ = " XDP RX-queue info extract example\n\n"
#include <getopt.h>
#include <net/if.h>
#include <time.h>
-
+#include <limits.h>
#include <arpa/inet.h>
#include <linux/if_link.h>
@@ -44,6 +44,9 @@ static struct bpf_map *rx_queue_index_map;
#define EXIT_FAIL_BPF 4
#define EXIT_FAIL_MEM 5
+#define FAIL_MEM_SIG INT_MAX
+#define FAIL_STAT_SIG (INT_MAX - 1)
+
static const struct option long_options[] = {
{"help", no_argument, NULL, 'h' },
{"dev", required_argument, NULL, 'd' },
@@ -77,6 +80,12 @@ static void int_exit(int sig)
printf("program on interface changed, not removing\n");
}
}
+
+ if (sig == FAIL_MEM_SIG)
+ exit(EXIT_FAIL_MEM);
+ else if (sig == FAIL_STAT_SIG)
+ exit(EXIT_FAIL);
+
exit(EXIT_OK);
}
@@ -141,7 +150,7 @@ static char* options2str(enum cfg_options_flags flag)
if (flag & READ_MEM)
return "read";
fprintf(stderr, "ERR: Unknown config option flags");
- exit(EXIT_FAIL);
+ int_exit(FAIL_STAT_SIG);
}
static void usage(char *argv[])
@@ -174,7 +183,7 @@ static __u64 gettime(void)
res = clock_gettime(CLOCK_MONOTONIC, &t);
if (res < 0) {
fprintf(stderr, "Error with gettimeofday! (%i)\n", res);
- exit(EXIT_FAIL);
+ int_exit(FAIL_STAT_SIG);
}
return (__u64) t.tv_sec * NANOSEC_PER_SEC + t.tv_nsec;
}
@@ -202,7 +211,7 @@ static struct datarec *alloc_record_per_cpu(void)
array = calloc(nr_cpus, sizeof(struct datarec));
if (!array) {
fprintf(stderr, "Mem alloc error (nr_cpus:%u)\n", nr_cpus);
- exit(EXIT_FAIL_MEM);
+ int_exit(FAIL_MEM_SIG);
}
return array;
}
@@ -215,7 +224,7 @@ static struct record *alloc_record_per_rxq(void)
array = calloc(nr_rxqs, sizeof(struct record));
if (!array) {
fprintf(stderr, "Mem alloc error (nr_rxqs:%u)\n", nr_rxqs);
- exit(EXIT_FAIL_MEM);
+ int_exit(FAIL_MEM_SIG);
}
return array;
}
@@ -229,7 +238,7 @@ static struct stats_record *alloc_stats_record(void)
rec = calloc(1, sizeof(struct stats_record));
if (!rec) {
fprintf(stderr, "Mem alloc error\n");
- exit(EXIT_FAIL_MEM);
+ int_exit(FAIL_MEM_SIG);
}
rec->rxq = alloc_record_per_rxq();
for (i = 0; i < nr_rxqs; i++)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/5] net: mdio: Mask PHY only when its ACPI node is present
From: Kai-Heng Feng @ 2022-04-21 12:18 UTC (permalink / raw)
To: Andrew Lunn
Cc: hkallweit1, linux, peppe.cavallaro, alexandre.torgue, joabreu,
davem, kuba, pabeni, netdev, linux-kernel
In-Reply-To: <YmFCxnYdRnnk41QQ@lunn.ch>
On Thu, Apr 21, 2022 at 7:40 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Apr 21, 2022 at 10:58:40AM +0800, Kai-Heng Feng wrote:
> > On Wed, Apr 20, 2022 at 10:47 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > On Wed, Apr 20, 2022 at 08:40:48PM +0800, Kai-Heng Feng wrote:
> > > > Not all PHY has an ACPI node, for those nodes auto probing is still
> > > > needed.
> > >
> > > Why do you need this?
> > >
> > > Documentation/firmware-guide/acpi/dsd/phy.rst
> > >
> > > There is nothing here about there being PHYs which are not listed in
> > > ACPI. If you have decided to go the ACPI route, you need to list the
> > > PHYs.
> >
> > This is for backward-compatibility. MAC can have ACPI node but PHY may
> > not have one.
>
> And if the PHY does not have an ACPI node, fall back to
> mdiobus_register(). This is what of_mdiobus_register() does. If
> np=NULL, it calls mdiobus_register() and skips all the OF stuff.
The equivalent to this scenario is that when MAC doesn't have ACPI node.
But yes it can unmask the PHYs if no ACPI node is found, then call
mdiobus_register().
Kai-Heng
>
> Andrew
^ permalink raw reply
* Re: [PATCH 4/5] net: phy: marvell: Add LED accessors for Marvell 88E1510
From: Kai-Heng Feng @ 2022-04-21 12:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: hkallweit1, linux, peppe.cavallaro, alexandre.torgue, joabreu,
davem, kuba, pabeni, netdev, linux-kernel
In-Reply-To: <YmFFWd42Nol7Lrlm@lunn.ch>
On Thu, Apr 21, 2022 at 7:51 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > This is not feasible.
> > If BIOS can define a method and restore the LED by itself, it can put
> > the method inside its S3 method and I don't have to work on this at
> > the first place.
>
> So maybe just declare the BIOS as FUBAR and move on to the next issue
> assigned to you.
>
> Do we really want the maintenance burden of this code for one machines
> BIOS?
Wasn't this the "set precedence" we discussed earlier for? Someone has
to be the first, and more users will leverage the new property we
added.
> Maybe the better solution is to push back on the vendor and its
> BIOS, tell them how they should of done this, if the BIOS wants to be
> in control of the LEDs it needs to offer the methods to control the
> LEDs. And then hopefully the next machine the vendor produces will
> have working BIOS.
The BIOS doesn't want to control the LED. It just provides a default
LED setting suitable for this platform, so the driver can use this
value over the hardcoded one in marvell phy driver.
So this really has nothing to do with with any ACPI method.
I believe the new property can be useful for DT world too.
>
> Your other option is to take part in the effort to add control of the
> LEDs via the standard Linux LED subsystem. The Marvel PHY driver is
> likely to be one of the first to gain support this for. So you can
> then totally take control of the LED from the BIOS and put it in the
> users hands. And such a solution will be applicable to many machines,
> not just one.
This series just wants to use the default value platform firmware provides.
Create a sysfs to let user meddle with LED value doesn't really help
the case here.
Kai-Heng
>
> Andrew
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: net: adin: document phy clock output properties
From: Andrew Lunn @ 2022-04-21 12:24 UTC (permalink / raw)
To: Josua Mayer
Cc: netdev, alvaro.karsz, Michael Hennerich, David S. Miller,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Alexandru Ardelean
In-Reply-To: <20220419102709.26432-2-josua@solid-run.com>
On Tue, Apr 19, 2022 at 01:27:07PM +0300, Josua Mayer wrote:
> The ADIN1300 supports generating certain clocks on its GP_CLK pin, as
> well as providing the reference clock on CLK25_REF.
>
> Add DT properties to configure both pins.
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 3/3] ARM: dts: imx6qdl-sr-som: update phy configuration for som revision 1.9
From: Andrew Lunn @ 2022-04-21 12:27 UTC (permalink / raw)
To: Josua Mayer
Cc: netdev, alvaro.karsz, Russell King, Rob Herring,
Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team
In-Reply-To: <20220419102709.26432-4-josua@solid-run.com>
On Tue, Apr 19, 2022 at 01:27:09PM +0300, Josua Mayer wrote:
> Since SoM revision 1.9 the PHY has been replaced with an ADIN1300,
> add an entry for it next to the original.
>
> Co-developed-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
> Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
> V1 -> V2: changed dts property name
>
> arch/arm/boot/dts/imx6qdl-sr-som.dtsi | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6qdl-sr-som.dtsi b/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
> index f86efd0ccc40..d46182095d79 100644
> --- a/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
> @@ -83,6 +83,12 @@ ethernet-phy@4 {
> qca,clk-out-frequency = <125000000>;
> qca,smarteee-tw-us-1g = <24>;
> };
> +
> + /* ADIN1300 (som rev 1.9 or later) */
> + ethernet-phy@1 {
> + reg = <1>;
> + adi,phy-output-clock = "125mhz-free-running";
> + };
There is currently the comment:
* The PHY can appear at either address 0 or 4 due to the
* configuration (LED) pin not being pulled sufficiently.
*/
It would be good to add another comment about this PHY at address 1.
Andrew
^ permalink raw reply
* Re: [syzbot] UBSAN: shift-out-of-bounds in ntfs_fill_super
From: syzbot @ 2022-04-21 12:29 UTC (permalink / raw)
To: almaz.alexandrovich, chao, davem, jaegeuk, johan.hedberg, kuba,
linux-bluetooth, linux-f2fs-devel, linux-kernel, llvm, luiz.dentz,
marcel, nathan, ndesaulniers, netdev, ntfs3, syzkaller-bugs, trix
In-Reply-To: <000000000000f8b5ef05dd25b963@google.com>
syzbot has bisected this issue to:
commit adf9ea89c719c1d23794e363f631e376b3ff8cbc
Author: Chao Yu <chao@kernel.org>
Date: Thu Aug 26 02:03:15 2021 +0000
f2fs: fix unexpected ENOENT comes from f2fs_map_blocks()
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=101dd0fcf00000
start commit: b253435746d9 Merge tag 'xtensa-20220416' of https://github..
git tree: upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=121dd0fcf00000
console output: https://syzkaller.appspot.com/x/log.txt?x=141dd0fcf00000
kernel config: https://syzkaller.appspot.com/x/.config?x=ff9f8140cbb3af7
dashboard link: https://syzkaller.appspot.com/bug?extid=1631f09646bc214d2e76
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12e13cfcf00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=135e3008f00000
Reported-by: syzbot+1631f09646bc214d2e76@syzkaller.appspotmail.com
Fixes: adf9ea89c719 ("f2fs: fix unexpected ENOENT comes from f2fs_map_blocks()")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: Ethernet TX buffer crossing 4K boundary?
From: Andrew Lunn @ 2022-04-21 12:32 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: netdev@vger.kernel.org, Eric Gratorp
In-Reply-To: <7e3fa36a3e16aca6fd7d00cadeeba8a8d71ceb0d.camel@infinera.com>
On Wed, Apr 20, 2022 at 09:09:58PM +0000, Joakim Tjernlund wrote:
> We have this custom Ethernet controller that cannot DMA a buffer if the buffer crosses 4K boundary.
> Any ideas how to deal with that limitation in the driver?
Does the DMA support scatter gather? You might be able to tweak the
generic scatter gather code to generate two blocks if it crosses the
boundary.
Otherwise, maybe look at the DMA bounce buffer code. It is normally
used when the DMA is limited in its address range, and the buffer
needs copying to another address. Maybe you can add a special mode
where it looks at if a 4K page is cross and then makes use of a bounce
buffer.
Andrew
^ permalink raw reply
* Re: [PATCH] drivers: net: davinci_mdio: using pm_runtime_resume_and_get instead of pm_runtime_get_sync
From: patchwork-bot+netdevbpf @ 2022-04-21 12:40 UTC (permalink / raw)
To: Lv Ruyi
Cc: grygorii.strashko, davem, kuba, linux-omap, netdev, linux-kernel,
chi.minghao, zealci
In-Reply-To: <20220418062921.2557884-1-chi.minghao@zte.com.cn>
Hello:
This patch was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:
On Mon, 18 Apr 2022 06:29:21 +0000 you wrote:
> From: Minghao Chi <chi.minghao@zte.com.cn>
>
> Using pm_runtime_resume_and_get is more appropriate
> for simplifing code
>
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
>
> [...]
Here is the summary with links:
- drivers: net: davinci_mdio: using pm_runtime_resume_and_get instead of pm_runtime_get_sync
https://git.kernel.org/netdev/net-next/c/4facbe3d4426
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [next] LTP: netns_breakns: Command \"add\" is unknown, try \"ip link help\".
From: Naresh Kamboju @ 2022-04-21 12:42 UTC (permalink / raw)
To: LTP List, Linux-Next Mailing List, open list, Netdev
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Stephen Rothwell,
vadimp@nvidia.com, idosch, Raju.Lakkaraju, jiri
Regressions found on all devices LTP containers test cases failed on
Linux next-20220420. [1]
- ltp-containers-tests/netns_comm_ns_exec_ipv6_ioctl
- ltp-containers-tests/netns_breakns_ns_exec_ipv6_netlink
- ltp-containers-tests/netns_breakns_ip_ipv6_netlink
- ltp-containers-tests/netns_breakns_ns_exec_ipv4_ioctl
- ltp-containers-tests/netns_breakns_ip_ipv4_netlink
- ltp-containers-tests/netns_comm_ip_ipv6_ioctl
- ltp-containers-tests/netns_comm_ip_ipv4_netlink
- ltp-containers-tests/netns_comm_ns_exec_ipv4_netlink
- ltp-containers-tests/netns_breakns_ns_exec_ipv6_ioctl
- ltp-containers-tests/netns_comm_ip_ipv6_netlink
- ltp-containers-tests/netns_comm_ns_exec_ipv4_ioctl
- ltp-containers-tests/netns_breakns_ns_exec_ipv4_netlink
- ltp-containers-tests/netns_breakns_ip_ipv4_ioctl
- ltp-containers-tests/netns_comm_ip_ipv4_ioctl
- ltp-containers-tests/netns_breakns_ip_ipv6_ioctl
- ltp-containers-tests/netns_comm_ns_exec_ipv6_netlink
Test log:
---------
netns_breakns 1 TINFO: timeout per run is 0h 15m 0s
Command \"add\" is unknown, try \"ip link help\".
netns_breakns 1 TBROK: unable to create veth pair devices
Command \"delete\" is unknown, try \"ip link help\".
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
metadata:
git_ref: master
git_repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next
git_sha: f1244c81da13009dbf61cb807f45881501c44789
git_describe: next-20220420
kernel_version: 5.18.0-rc3
kernel-config: https://builds.tuxbuild.com/283Ot2o4P4hh7rNSH56BnbPbNba/config
build-url: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next/-/pipelines/520334286
artifact-location: https://builds.tuxbuild.com/283Ot2o4P4hh7rNSH56BnbPbNba
I will bisect these failures.
--
Linaro LKFT
https://lkft.linaro.org
[1] https://lkft.validation.linaro.org/scheduler/job/4925635#L1272
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox