RE: [PATCH v4 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Monday, March 12, 2018 4:37 PM
> To: Razvan Stefanescu <razvan.stefane...@nxp.com>
> Cc: gre...@linuxfoundation.org; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Alexander Graf
> <ag...@suse.de>; a...@arndb.de; Alexandru Marginean
> <alexandru.margin...@nxp.com>; Ruxandra Ioana Ciocoi Radulescu
> <ruxandra.radule...@nxp.com>; Ioana Ciornei <ioana.cior...@nxp.com>;
> Laurentiu Tudor <laurentiu.tu...@nxp.com>; stuyo...@gmail.com
> Subject: Re: [PATCH v4 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2
> Ethernet Switch driver
>
> > +static int port_netdevice_event(struct notifier_block *unused,
> > + unsigned long event, void *ptr)
> > +{
> > + struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> > + struct netdev_notifier_changeupper_info *info = ptr;
> > + struct net_device *upper_dev;
> > + int err = 0;
> > +
> > + if (netdev->netdev_ops != _port_ops)
> > + return NOTIFY_DONE;
> > +
> > + /* Handle just upper dev link/unlink for the moment */
> > + if (event == NETDEV_CHANGEUPPER) {
> > + upper_dev = info->upper_dev;
> > + if (netif_is_bridge_master(upper_dev)) {
> > + if (info->linking)
> > + err = port_bridge_join(netdev);
> > + else
> > + err = port_bridge_leave(netdev);
> > + }
> > + }
> > +
> > + return notifier_from_errno(err);
> > +}
>
> I could be missing something here, but don't you need to pass to
> port_bridge_join() which bridge the port is joining. There can be
> multiple bridges, so you need to ensure the port joins the correct
> bridge.
>
Thank you for noticing this. I'll add proper checks in next version.
Razvan
> Andrew
RE: [PATCH v4 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Monday, March 12, 2018 4:37 PM
> To: Razvan Stefanescu
> Cc: gre...@linuxfoundation.org; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Alexander Graf
> ; a...@arndb.de; Alexandru Marginean
> ; Ruxandra Ioana Ciocoi Radulescu
> ; Ioana Ciornei ;
> Laurentiu Tudor ; stuyo...@gmail.com
> Subject: Re: [PATCH v4 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2
> Ethernet Switch driver
>
> > +static int port_netdevice_event(struct notifier_block *unused,
> > + unsigned long event, void *ptr)
> > +{
> > + struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> > + struct netdev_notifier_changeupper_info *info = ptr;
> > + struct net_device *upper_dev;
> > + int err = 0;
> > +
> > + if (netdev->netdev_ops != _port_ops)
> > + return NOTIFY_DONE;
> > +
> > + /* Handle just upper dev link/unlink for the moment */
> > + if (event == NETDEV_CHANGEUPPER) {
> > + upper_dev = info->upper_dev;
> > + if (netif_is_bridge_master(upper_dev)) {
> > + if (info->linking)
> > + err = port_bridge_join(netdev);
> > + else
> > + err = port_bridge_leave(netdev);
> > + }
> > + }
> > +
> > + return notifier_from_errno(err);
> > +}
>
> I could be missing something here, but don't you need to pass to
> port_bridge_join() which bridge the port is joining. There can be
> multiple bridges, so you need to ensure the port joins the correct
> bridge.
>
Thank you for noticing this. I'll add proper checks in next version.
Razvan
> Andrew
Re: [PATCH v4 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> +static int port_netdevice_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
> + struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> + struct netdev_notifier_changeupper_info *info = ptr;
> + struct net_device *upper_dev;
> + int err = 0;
> +
> + if (netdev->netdev_ops != _port_ops)
> + return NOTIFY_DONE;
> +
> + /* Handle just upper dev link/unlink for the moment */
> + if (event == NETDEV_CHANGEUPPER) {
> + upper_dev = info->upper_dev;
> + if (netif_is_bridge_master(upper_dev)) {
> + if (info->linking)
> + err = port_bridge_join(netdev);
> + else
> + err = port_bridge_leave(netdev);
> + }
> + }
> +
> + return notifier_from_errno(err);
> +}
I could be missing something here, but don't you need to pass to
port_bridge_join() which bridge the port is joining. There can be
multiple bridges, so you need to ensure the port joins the correct
bridge.
Andrew
Re: [PATCH v4 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
> +static int port_netdevice_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
> + struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> + struct netdev_notifier_changeupper_info *info = ptr;
> + struct net_device *upper_dev;
> + int err = 0;
> +
> + if (netdev->netdev_ops != _port_ops)
> + return NOTIFY_DONE;
> +
> + /* Handle just upper dev link/unlink for the moment */
> + if (event == NETDEV_CHANGEUPPER) {
> + upper_dev = info->upper_dev;
> + if (netif_is_bridge_master(upper_dev)) {
> + if (info->linking)
> + err = port_bridge_join(netdev);
> + else
> + err = port_bridge_leave(netdev);
> + }
> + }
> +
> + return notifier_from_errno(err);
> +}
I could be missing something here, but don't you need to pass to
port_bridge_join() which bridge the port is joining. There can be
multiple bridges, so you need to ensure the port joins the correct
bridge.
Andrew
Re: [PATCH v4 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
On Mon, Mar 12, 2018 at 03:49:51AM -0500, Razvan Stefanescu wrote:
> +static irqreturn_t ethsw_irq0_handler(int irq_num, void *arg)
> +{
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static int ethsw_setup_irqs(struct fsl_mc_device *sw_dev)
> +{
> + struct device *dev = _dev->dev;
> + struct ethsw_core *ethsw = dev_get_drvdata(dev);
> + u32 mask = DPSW_IRQ_EVENT_LINK_CHANGED;
> + struct fsl_mc_device_irq *irq;
> + int err;
> +
> + err = fsl_mc_allocate_irqs(sw_dev);
> + if (err) {
> + dev_err(dev, "MC irqs allocation failed\n");
> + return err;
> + }
> +
> + if (WARN_ON(sw_dev->obj_desc.irq_count != DPSW_IRQ_NUM)) {
> + err = -EINVAL;
> + goto free_irq;
> + }
> +
> + err = dpsw_set_irq_enable(ethsw->mc_io, 0, ethsw->dpsw_handle,
> + DPSW_IRQ_INDEX_IF, 0);
> + if (err) {
> + dev_err(dev, "dpsw_set_irq_enable err %d\n", err);
> + goto free_irq;
> + }
> +
> + irq = sw_dev->irqs[DPSW_IRQ_INDEX_IF];
> +
> + err = devm_request_threaded_irq(dev, irq->msi_desc->irq,
> + ethsw_irq0_handler,
> + ethsw_irq0_handler_thread,
> + IRQF_NO_SUSPEND | IRQF_ONESHOT,
> + dev_name(dev), dev);
Hi Razvan
You can pass NULL instead of ethsw_irq0_handler.
Andrew
Re: [PATCH v4 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
On Mon, Mar 12, 2018 at 03:49:51AM -0500, Razvan Stefanescu wrote:
> +static irqreturn_t ethsw_irq0_handler(int irq_num, void *arg)
> +{
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static int ethsw_setup_irqs(struct fsl_mc_device *sw_dev)
> +{
> + struct device *dev = _dev->dev;
> + struct ethsw_core *ethsw = dev_get_drvdata(dev);
> + u32 mask = DPSW_IRQ_EVENT_LINK_CHANGED;
> + struct fsl_mc_device_irq *irq;
> + int err;
> +
> + err = fsl_mc_allocate_irqs(sw_dev);
> + if (err) {
> + dev_err(dev, "MC irqs allocation failed\n");
> + return err;
> + }
> +
> + if (WARN_ON(sw_dev->obj_desc.irq_count != DPSW_IRQ_NUM)) {
> + err = -EINVAL;
> + goto free_irq;
> + }
> +
> + err = dpsw_set_irq_enable(ethsw->mc_io, 0, ethsw->dpsw_handle,
> + DPSW_IRQ_INDEX_IF, 0);
> + if (err) {
> + dev_err(dev, "dpsw_set_irq_enable err %d\n", err);
> + goto free_irq;
> + }
> +
> + irq = sw_dev->irqs[DPSW_IRQ_INDEX_IF];
> +
> + err = devm_request_threaded_irq(dev, irq->msi_desc->irq,
> + ethsw_irq0_handler,
> + ethsw_irq0_handler_thread,
> + IRQF_NO_SUSPEND | IRQF_ONESHOT,
> + dev_name(dev), dev);
Hi Razvan
You can pass NULL instead of ethsw_irq0_handler.
Andrew
