1. 02 Feb, 2024 9 commits
    • Christian Marangi's avatar
      dt-bindings: net: ipq4019-mdio: document now supported clock-frequency · 9484b955
      Christian Marangi authored
      Document support for clock-frequency and add details on why this
      property is needed and what values are supported.
      
      From internal documentation, while other values are supported, the
      correct function of the MDIO bus is not assured hence add only the
      suggested supported values to the property enum.
      Signed-off-by: default avatarChristian Marangi <ansuelsmth@gmail.com>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      9484b955
    • Jakub Kicinski's avatar
      Merge branch 'net-ipa-simplify-tx-power-handling' · 747056a9
      Jakub Kicinski authored
      Alex Elder says:
      
      ====================
      net: ipa: simplify TX power handling
      
      In order to deliver a packet to the IPA hardware, we must ensure
      it is powered.  We request power by calling pm_runtime_get(), and
      its return value tells us the power state.  We can't block in
      ipa_start_xmit(), so if power isn't enabled we prevent further
      transmit attempts by calling netif_stop_queue().  Power will
      eventually become enabled, at which point we call netif_wake_queue()
      to allow the transmit to be retried.  When it does, the power should
      be enabled, so the packet delivery can proceed.
      
      The logic that handles this is convoluted, and was put in place
      to address a race condition pointed out by Jakub Kicinski during
      review.  The fix addressed that race, as well as another one that
      was found while investigating it:
        b8e36e13 ("net: ipa: fix TX queue race")
      I have wanted to simplify this code ever since, and I'm pleased to
      report that this series implements a much better solution that
      avoids both race conditions.
      
      The first race occurs between the ->ndo_start_xmit thread and the
      runtime resume thread.  If we find power is not enabled when
      requested in ipa_start_xmit(), we stop queueing.  But there's a
      chance the runtime resume will enable queuing just before that,
      leaving queueing stopped forever.  A flag is used to ensure that
      does not occur.
      
      A second flag is used to prevent NETDEV_TX_BUSY from being returned
      repeatedly during the small window between enabling queueing and
      finishing power resume.  This can happen if resume was underway when
      pm_runtime_get() was called and completes immediately afterward.
      This condition only exists because of the use of the first flag.
      
      The fix is to disable transmit for *every* call to ipa_start_xmit(),
      disabling it *before* calling pm_runtime_get().  This leaves three
      cases:
        - If the return value indicates power is not active (or is in
          transition), queueing remains disabled--thereby avoiding
          the race between disabling it and a concurrent power thread
          enabling it.
        - If pm_runtime_get() returns an error, we drop the packet and
          re-enable queueing.
        - Finally, if the hardware is powered, we re-enable queueing
          before delivering the packet to the hardware.
      
      So the first race is avoided.  And as a result, the second condition
      will not occur.
      
      The first patch adds pointers to the TX and RX IPA endpoints in the
      netdev private data.  The second patch has netif_stop_queue() be
      called for all packets; if pm_runtime_get() indicates power is
      enabled (or an error), netif_wake_queue() is called to enable it
      again.  The third and fourth patches get rid of the STARTED and
      STOPPED IPA power flags, as well as the power spinlock, because they
      are no longer needed.  The last three patches just eliminate some
      trivial functions, open-coding them instead.
      ====================
      
      Link: https://lore.kernel.org/r/20240130192305.250915-1-elder@linaro.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      747056a9
    • Alex Elder's avatar
      net: ipa: kill ipa_power_modem_queue_wake() · e01bbdc9
      Alex Elder authored
      All ipa_power_modem_queue_wake() does is call netif_wake_queue()
      on the modem netdev.  There is no need to wrap that call in a
      trivial function (and certainly not one defined in "ipa_power.c").
      
      So get rid of ipa_power_modem_queue_wake(), and replace its one
      caller with a direct call to netif_wake_queue().  Determine the
      netdev pointer to use from the private TX endpoint's netdev pointer.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Link: https://lore.kernel.org/r/20240130192305.250915-8-elder@linaro.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      e01bbdc9
    • Alex Elder's avatar
      net: ipa: kill ipa_power_modem_queue_active() · 2acf5fc8
      Alex Elder authored
      All ipa_power_modem_queue_active() does now is call netif_wake_queue().
      Just call netif_wake_queue() in the two places it's needed, and get
      rid of ipa_power_modem_queue_active().
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Link: https://lore.kernel.org/r/20240130192305.250915-7-elder@linaro.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      2acf5fc8
    • Alex Elder's avatar
      net: ipa: kill ipa_power_modem_queue_stop() · 30cdaea2
      Alex Elder authored
      All ipa_power_modem_queue_stop() does now is call netif_stop_queue().
      Just call netif_stop_queue() in the one place it's needed, and get
      rid of ipa_power_modem_queue_stop().
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Link: https://lore.kernel.org/r/20240130192305.250915-6-elder@linaro.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      30cdaea2
    • Alex Elder's avatar
      net: ipa: kill the IPA power STOPPED flag · 86c9a492
      Alex Elder authored
      Currently the STOPPED IPA power flag is used to indicate that the
      transmit queue has been stopped.  Previously this was used to avoid
      setting the STARTED flag unless the queue had already been stopped.
      It meant transmit queuing would be enabled on resume if it was
      stopped by the transmit path--and if so, it ensured it only got
      enabled once.
      
      We only stop the transmit queue in the transmit path.  The STARTED
      flag has been removed, and it causes no real harm to enable
      transmits when they're already enabled.  So we can get rid of
      the STOPPED flag and call netif_wake_queue() unconditionally.
      
      This makes the IPA power spinlock unnecessary, so it can be removed
      as well.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Link: https://lore.kernel.org/r/20240130192305.250915-5-elder@linaro.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      86c9a492
    • Alex Elder's avatar
      net: ipa: kill the STARTED IPA power flag · 688de12f
      Alex Elder authored
      A transmit on the modem netdev can only complete if the IPA hardware
      is powered.  Currently, if a transmit request arrives when the
      hardware was not powered, further transmits are be stopped to allow
      power-up to complete.  Once power-up completes, transmits are once
      again enabled.
      
      Runtime resume can complete at the same time a transmit request is
      being handled, and previously there was a race between stopping and
      restarting transmits.  The STARTED flag was used to ensure the
      stop request in the transmit path was skipped if the start request
      in the runtime resume path had already occurred.
      
      Now, the queue is *always* stopped in the transmit path, *before*
      determining whether power is ACTIVE.  If power is found to already
      be active (or if the socket buffer is gets dropped), transmit is
      re-enabled.  Otherwise it will (always) be enabled after runtime
      resume completes.
      
      The race between transmit and runtime resume no longer exists, so
      there is no longer any need to maintain the STARTED flag.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Link: https://lore.kernel.org/r/20240130192305.250915-4-elder@linaro.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      688de12f
    • Alex Elder's avatar
      net: ipa: begin simplifying TX queue stop · 844ecc4a
      Alex Elder authored
      There are a number of flags used in the IPA driver to attempt to
      manage race conditions that can occur between runtime resume and
      netdev transmit.  If we disable TX before requesting power, we can
      avoid these races entirely, simplifying things considerably.
      
      This patch implements the main change, disabling transmit always in
      the net_device->ndo_start_xmit() callback, then re-enabling it again
      whenever we find power is active (or when we drop the skb).
      
      The patches that follow will refactor the "old" code to the point
      that most of it can be eliminated.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Link: https://lore.kernel.org/r/20240130192305.250915-3-elder@linaro.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      844ecc4a
    • Alex Elder's avatar
      net: ipa: stash modem TX and RX endpoints · 102c28b8
      Alex Elder authored
      Rather than repeatedly looking up the endpoints in the name map,
      save the modem TX and RX endpoint pointers in the netdev private
      area.
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Link: https://lore.kernel.org/r/20240130192305.250915-2-elder@linaro.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      102c28b8
  2. 01 Feb, 2024 31 commits