* [PATCH for-next 11/11] IB/hns: Fix for Checkpatch.pl comment style errors
From: Salil Mehta @ 2016-11-04 16:36 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: salil.mehta-hv44wF8Li93QT0dZR+AlfA,
xavier.huwei-hv44wF8Li93QT0dZR+AlfA,
oulijun-hv44wF8Li93QT0dZR+AlfA,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <20161104163633.141880-1-salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
This patch correct the comment style errors caught by
checkpatch.pl script
Signed-off-by: Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/hw/hns/hns_roce_cmd.c | 8 ++--
drivers/infiniband/hw/hns/hns_roce_device.h | 28 +++++++-------
drivers/infiniband/hw/hns/hns_roce_eq.c | 6 +--
drivers/infiniband/hw/hns/hns_roce_hem.c | 6 +--
drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 56 +++++++++++++--------------
drivers/infiniband/hw/hns/hns_roce_main.c | 28 +++++++-------
6 files changed, 66 insertions(+), 66 deletions(-)
diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c b/drivers/infiniband/hw/hns/hns_roce_cmd.c
index 2a0b6c0..8c1f7a6 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
@@ -216,10 +216,10 @@ static int __hns_roce_cmd_mbox_wait(struct hns_roce_dev *hr_dev, u64 in_param,
goto out;
/*
- * It is timeout when wait_for_completion_timeout return 0
- * The return value is the time limit set in advance
- * how many seconds showing
- */
+ * It is timeout when wait_for_completion_timeout return 0
+ * The return value is the time limit set in advance
+ * how many seconds showing
+ */
if (!wait_for_completion_timeout(&context->done,
msecs_to_jiffies(timeout))) {
dev_err(dev, "[cmd]wait_for_completion_timeout timeout\n");
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 9ef1cc3..e48464d 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -201,9 +201,9 @@ struct hns_roce_bitmap {
/* Order = 0: bitmap is biggest, order = max bitmap is least (only a bit) */
/* Every bit repesent to a partner free/used status in bitmap */
/*
-* Initial, bits of other bitmap are all 0 except that a bit of max_order is 1
-* Bit = 1 represent to idle and available; bit = 0: not available
-*/
+ * Initial, bits of other bitmap are all 0 except that a bit of max_order is 1
+ * Bit = 1 represent to idle and available; bit = 0: not available
+ */
struct hns_roce_buddy {
/* Members point to every order level bitmap */
unsigned long **bits;
@@ -365,25 +365,25 @@ struct hns_roce_cmdq {
struct mutex hcr_mutex;
struct semaphore poll_sem;
/*
- * Event mode: cmd register mutex protection,
- * ensure to not exceed max_cmds and user use limit region
- */
+ * Event mode: cmd register mutex protection,
+ * ensure to not exceed max_cmds and user use limit region
+ */
struct semaphore event_sem;
int max_cmds;
spinlock_t context_lock;
int free_head;
struct hns_roce_cmd_context *context;
/*
- * Result of get integer part
- * which max_comds compute according a power of 2
- */
+ * Result of get integer part
+ * which max_comds compute according a power of 2
+ */
u16 token_mask;
/*
- * Process whether use event mode, init default non-zero
- * After the event queue of cmd event ready,
- * can switch into event mode
- * close device, switch into poll mode(non event mode)
- */
+ * Process whether use event mode, init default non-zero
+ * After the event queue of cmd event ready,
+ * can switch into event mode
+ * close device, switch into poll mode(non event mode)
+ */
u8 use_events;
u8 toggle;
};
diff --git a/drivers/infiniband/hw/hns/hns_roce_eq.c b/drivers/infiniband/hw/hns/hns_roce_eq.c
index 21e21b0..50f8649 100644
--- a/drivers/infiniband/hw/hns/hns_roce_eq.c
+++ b/drivers/infiniband/hw/hns/hns_roce_eq.c
@@ -371,9 +371,9 @@ static int hns_roce_aeq_ovf_int(struct hns_roce_dev *hr_dev,
int i = 0;
/**
- * AEQ overflow ECC mult bit err CEQ overflow alarm
- * must clear interrupt, mask irq, clear irq, cancel mask operation
- */
+ * AEQ overflow ECC mult bit err CEQ overflow alarm
+ * must clear interrupt, mask irq, clear irq, cancel mask operation
+ */
aeshift_val = roce_read(hr_dev, ROCEE_CAEP_AEQC_AEQE_SHIFT_REG);
if (roce_get_bit(aeshift_val,
diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.c b/drivers/infiniband/hw/hns/hns_roce_hem.c
index 250d8f2..c5104e0 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hem.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hem.c
@@ -80,9 +80,9 @@ struct hns_roce_hem *hns_roce_alloc_hem(struct hns_roce_dev *hr_dev, int npages,
--order;
/*
- * Alloc memory one time. If failed, don't alloc small block
- * memory, directly return fail.
- */
+ * Alloc memory one time. If failed, don't alloc small block
+ * memory, directly return fail.
+ */
mem = &chunk->mem[chunk->npages];
buf = dma_alloc_coherent(&hr_dev->pdev->dev, PAGE_SIZE << order,
&sg_dma_address(mem), gfp_mask);
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 76edebe..8107f8c 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -1352,9 +1352,9 @@ static void __hns_roce_v1_cq_clean(struct hns_roce_cq *hr_cq, u32 qpn,
}
/*
- * Now backwards through the CQ, removing CQ entries
- * that match our QP by overwriting them with next entries.
- */
+ * Now backwards through the CQ, removing CQ entries
+ * that match our QP by overwriting them with next entries.
+ */
while ((int) --prod_index - (int) hr_cq->cons_index >= 0) {
cqe = get_cqe(hr_cq, prod_index & hr_cq->ib_cq.cqe);
if ((roce_get_field(cqe->cqe_byte_16, CQE_BYTE_16_LOCAL_QPN_M,
@@ -1376,9 +1376,9 @@ static void __hns_roce_v1_cq_clean(struct hns_roce_cq *hr_cq, u32 qpn,
if (nfreed) {
hr_cq->cons_index += nfreed;
/*
- * Make sure update of buffer contents is done before
- * updating consumer index.
- */
+ * Make sure update of buffer contents is done before
+ * updating consumer index.
+ */
wmb();
hns_roce_v1_cq_set_ci(hr_cq, hr_cq->cons_index);
@@ -1473,7 +1473,7 @@ void hns_roce_v1_write_cqc(struct hns_roce_dev *hr_dev,
roce_set_bit(cq_context->cqc_byte_32,
CQ_CQNTEXT_CQC_BYTE_32_TYPE_OF_COMPLETION_NOTIFICATION_S,
0);
- /*The initial value of cq's ci is 0 */
+ /* The initial value of cq's ci is 0 */
roce_set_field(cq_context->cqc_byte_32,
CQ_CONTEXT_CQC_BYTE_32_CQ_CONS_IDX_M,
CQ_CONTEXT_CQC_BYTE_32_CQ_CONS_IDX_S, 0);
@@ -1490,9 +1490,9 @@ int hns_roce_v1_req_notify_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags)
notification_flag = (flags & IB_CQ_SOLICITED_MASK) ==
IB_CQ_SOLICITED ? CQ_DB_REQ_NOT : CQ_DB_REQ_NOT_SOL;
/*
- * flags = 0; Notification Flag = 1, next
- * flags = 1; Notification Flag = 0, solocited
- */
+ * flags = 0; Notification Flag = 1, next
+ * flags = 1; Notification Flag = 0, solocited
+ */
doorbell[0] = hr_cq->cons_index & ((hr_cq->cq_depth << 1) - 1);
roce_set_bit(doorbell[1], ROCEE_DB_OTHERS_H_ROCEE_DB_OTH_HW_SYNS_S, 1);
roce_set_field(doorbell[1], ROCEE_DB_OTHERS_H_ROCEE_DB_OTH_CMD_M,
@@ -1647,10 +1647,10 @@ static int hns_roce_v1_poll_one(struct hns_roce_cq *hr_cq,
wq = &(*cur_qp)->sq;
if ((*cur_qp)->sq_signal_bits) {
/*
- * If sg_signal_bit is 1,
- * firstly tail pointer updated to wqe
- * which current cqe correspond to
- */
+ * If sg_signal_bit is 1,
+ * firstly tail pointer updated to wqe
+ * which current cqe correspond to
+ */
wqe_ctr = (u16)roce_get_field(cqe->cqe_byte_4,
CQE_BYTE_4_WQE_INDEX_M,
CQE_BYTE_4_WQE_INDEX_S);
@@ -2072,11 +2072,11 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
}
/*
- *Reset to init
- * Mandatory param:
- * IB_QP_STATE | IB_QP_PKEY_INDEX | IB_QP_PORT | IB_QP_ACCESS_FLAGS
- * Optional param: NA
- */
+ * Reset to init
+ * Mandatory param:
+ * IB_QP_STATE | IB_QP_PKEY_INDEX | IB_QP_PORT | IB_QP_ACCESS_FLAGS
+ * Optional param: NA
+ */
if (cur_state == IB_QPS_RESET && new_state == IB_QPS_INIT) {
roce_set_field(context->qpc_bytes_4,
QP_CONTEXT_QPC_BYTES_4_TRANSPORT_SERVICE_TYPE_M,
@@ -2584,9 +2584,9 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
}
/*
- * Use rst2init to instead of init2init with drv,
- * need to hw to flash RQ HEAD by DB again
- */
+ * Use rst2init to instead of init2init with drv,
+ * need to hw to flash RQ HEAD by DB again
+ */
if (cur_state == IB_QPS_INIT && new_state == IB_QPS_INIT) {
/* Memory barrier */
wmb();
@@ -2925,9 +2925,9 @@ static void hns_roce_v1_destroy_qp_common(struct hns_roce_dev *hr_dev,
if (hr_qp->ibqp.qp_type == IB_QPT_RC) {
if (hr_qp->state != IB_QPS_RESET) {
/*
- * Set qp to ERR,
- * waiting for hw complete processing all dbs
- */
+ * Set qp to ERR,
+ * waiting for hw complete processing all dbs
+ */
if (hns_roce_v1_qp_modify(hr_dev, NULL,
to_hns_roce_state(
(enum ib_qp_state)hr_qp->state),
@@ -2940,9 +2940,9 @@ static void hns_roce_v1_destroy_qp_common(struct hns_roce_dev *hr_dev,
sdbisusepr_val = roce_read(hr_dev,
ROCEE_SDB_ISSUE_PTR_REG);
/*
- * Query db process status,
- * until hw process completely
- */
+ * Query db process status,
+ * until hw process completely
+ */
end = msecs_to_jiffies(
HNS_ROCE_QP_DESTROY_TIMEOUT_MSECS) + jiffies;
do {
diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
index 795ef97..914d0ac 100644
--- a/drivers/infiniband/hw/hns/hns_roce_main.c
+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
@@ -148,8 +148,8 @@ static int handle_en_event(struct hns_roce_dev *hr_dev, u8 port,
break;
case NETDEV_DOWN:
/*
- * In v1 engine, only support all ports closed together.
- */
+ * In v1 engine, only support all ports closed together.
+ */
break;
default:
dev_dbg(dev, "NETDEV event = 0x%x!\n", (u32)(event));
@@ -773,10 +773,10 @@ static int hns_roce_init_hem(struct hns_roce_dev *hr_dev)
}
/**
-* hns_roce_setup_hca - setup host channel adapter
-* @hr_dev: pointer to hns roce device
-* Return : int
-*/
+ * hns_roce_setup_hca - setup host channel adapter
+ * @hr_dev: pointer to hns roce device
+ * Return : int
+ */
static int hns_roce_setup_hca(struct hns_roce_dev *hr_dev)
{
int ret;
@@ -841,11 +841,11 @@ static int hns_roce_setup_hca(struct hns_roce_dev *hr_dev)
}
/**
-* hns_roce_probe - RoCE driver entrance
-* @pdev: pointer to platform device
-* Return : int
-*
-*/
+ * hns_roce_probe - RoCE driver entrance
+ * @pdev: pointer to platform device
+ * Return : int
+ *
+ */
static int hns_roce_probe(struct platform_device *pdev)
{
int ret;
@@ -958,9 +958,9 @@ static int hns_roce_probe(struct platform_device *pdev)
}
/**
-* hns_roce_remove - remove RoCE device
-* @pdev: pointer to platform device
-*/
+ * hns_roce_remove - remove RoCE device
+ * @pdev: pointer to platform device
+ */
static int hns_roce_remove(struct platform_device *pdev)
{
struct hns_roce_dev *hr_dev = platform_get_drvdata(pdev);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH for-next 00/11] Code improvements & fixes for HNS RoCE driver
From: Salil Mehta @ 2016-11-04 16:36 UTC (permalink / raw)
To: dledford
Cc: salil.mehta, xavier.huwei, oulijun, mehta.salil.lnk, linux-rdma,
netdev, linux-kernel, linuxarm
This patchset introduces some code improvements and fixes
for the identified problems in the HNS RoCE driver.
Lijun Ou (4):
IB/hns: Add the interface for querying QP1
IB/hns: add self loopback for CM
IB/hns: Modify the condition of notifying hardware loopback
IB/hns: Fix the bug for qp state in hns_roce_v1_m_qp()
Salil Mehta (1):
IB/hns: Fix for Checkpatch.pl comment style errors
Shaobo Xu (1):
IB/hns: Implement the add_gid/del_gid and optimize the GIDs
management
Wei Hu (Xavier) (5):
IB/hns: Add code for refreshing CQ CI using TPTR
IB/hns: Optimize the logic of allocating memory using APIs
IB/hns: Modify the macro for the timeout when cmd process
IB/hns: Modify query info named port_num when querying RC QP
IB/hns: Change qpn allocation to round-robin mode.
drivers/infiniband/hw/hns/hns_roce_alloc.c | 11 +-
drivers/infiniband/hw/hns/hns_roce_cmd.c | 8 +-
drivers/infiniband/hw/hns/hns_roce_cmd.h | 7 +-
drivers/infiniband/hw/hns/hns_roce_common.h | 2 -
drivers/infiniband/hw/hns/hns_roce_cq.c | 17 +-
drivers/infiniband/hw/hns/hns_roce_device.h | 45 ++--
drivers/infiniband/hw/hns/hns_roce_eq.c | 6 +-
drivers/infiniband/hw/hns/hns_roce_hem.c | 6 +-
drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 271 +++++++++++++++++------
drivers/infiniband/hw/hns/hns_roce_hw_v1.h | 17 +-
drivers/infiniband/hw/hns/hns_roce_main.c | 311 +++++++--------------------
drivers/infiniband/hw/hns/hns_roce_mr.c | 21 +-
drivers/infiniband/hw/hns/hns_roce_pd.c | 5 +-
drivers/infiniband/hw/hns/hns_roce_qp.c | 2 +-
14 files changed, 367 insertions(+), 362 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: Ethernet not working on a different SoC with same eth HW
From: Florian Fainelli @ 2016-11-04 16:45 UTC (permalink / raw)
To: Måns Rullgård, Andrew Lunn
Cc: Mason, netdev, Timur Tabi, Sergei Shtylyov, Zefir Kurtisi,
Martin Blumenstingl, Uwe Kleine-Konig, Daniel Mack,
Sebastian Frias
In-Reply-To: <yw1xh97njmmw.fsf@unicorn.mansr.com>
On 11/04/2016 08:22 AM, Måns Rullgård wrote:
> Andrew Lunn <andrew@lunn.ch> writes:
>
>> On Fri, Nov 04, 2016 at 03:05:00PM +0000, Måns Rullgård wrote:
>>> Andrew Lunn <andrew@lunn.ch> writes:
>>>
>>>>>> I agree with you. But fixing it is likely to break boards which
>>>>>> currently have "rgmii", but actually need the delay in order to work.
>>>>>
>>>>> Does the internal delay here refer to the PHY or the MAC? It's a
>>>>> property of the MAC node after all.
>>>>
>>>> It is the PHY which applies the delay.
>>>
>>> Says who?
>>
>> The source code.
>
> There's source code that disagrees with that. The Broadcom GENET
> driver, for instance.
Correct, and in the case where the MAC adds the delay while transmitting
(because it supports that) the expectation is that the PHY would remove
such a delay internally, conversely, the PHY would introduce a delay
while transmitting back to the PHY, in order to produce the desired 90
degrees shift on the RGMII signals, and get reproduce the correct clock
and data alignment internally.
>
>>> Some MACs can do it too.
>>
>> I'm sure they can. But look at the code. Nearly none do, and those
>> that do are potentially broken.
>
> Those few drivers that do anything differently based on these values
> enable clock delay in the MAC. That's why I wrote the NB8800 driver the
> way I did.
>
I don't really what is wrong with the nb8800 driver at the moment, so
maybe this is just a configuration issue with the Atheros PHY driver,
it's not like it has not given people headache judging by the recent
discussions...
--
Florian
^ permalink raw reply
* Re: [PATCH 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level
From: Måns Rullgård @ 2016-11-04 16:49 UTC (permalink / raw)
To: Florian Fainelli
Cc: Sebastian Frias, David S. Miller, netdev, LKML, Mason,
Andrew Lunn
In-Reply-To: <b792b229-6126-c3a7-e071-bdd93fe51626@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 11/04/2016 08:36 AM, Sebastian Frias wrote:
>> Hi Måns,
>>
>> On 11/04/2016 04:18 PM, Måns Rullgård wrote:
>>> Sebastian Frias <sf84@laposte.net> writes:
>>>
>>>> The delay can be applied at PHY or MAC level, but since
>>>> PHY drivers will apply the delay at PHY level when using
>>>> one of the "internal delay" declinations of RGMII mode
>>>> (like PHY_INTERFACE_MODE_RGMII_TXID), applying it again
>>>> at MAC level causes issues.
>>>
>>> The Broadcom GENET driver does the same thing.
>>>
>>
>> Well, I don't know who uses that driver, or why they did it that way.
>
> I do use this driver and it works for me (tm), although I tested mostly
> with Broadcom PHYs and Ethernet switches, rarely with third party PHYs,
> but had that too, but all of that is in tree though,
> drivers/net/phy/broadcom.com, drivers/net/dsa/b53/ so feel free to
> "audit" that part of the code too.
>
> The configuration of the GENET port multiplexer requires us to specify
> how we want to align the clock and data, if we don't do that, and the
> PHY is also not agreeing with how its own delays should be configured,
> mayhem ensues, ranging from occasional transmit success, to high rates
> of CRC/FCS errors in best cases.
>
> I did verify that the settings were correct using a scope FWIW.
>
>>
>> However, with the current code and DT bindings, if one requires
>> the delay, phy-connection-type="rgmii-txid" must be set.
>
> Yes, and we would set it correctly for our Broadcom reference boards
> using this driver.
>
>>
>> But when doing so, both the Atheros 8035 and the Aurora NB8800 drivers
>> will apply the delay.
>>
>> I think a better way of dealing with this is that both, PHY and MAC
>> drivers exchange information so that the delay is applied only once.
>
> Exchange what information? The PHY device interface (phydev->interface)
> conveys the needed information for both entities.
There doesn't seem to be any consensus among the drivers regarding where
the delay should be applied. Since only a few drivers, MAC or PHY, act
on this property, most combinations still work by chance. It is common
for boards to set the delay at the PHY using external config pins so no
software setup is required (although I have one Sigma based board that
gets this wrong). I suspect if drivers/net/ethernet/broadcom/genet were
used with one of the four PHY drivers that also set the delay based on
this DT property, things would go wrong.
--
Måns Rullgård
^ permalink raw reply
* Re: Ethernet not working on a different SoC with same eth HW
From: Måns Rullgård @ 2016-11-04 16:55 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, Mason, netdev, Timur Tabi, Sergei Shtylyov,
Zefir Kurtisi, Martin Blumenstingl, Uwe Kleine-Konig, Daniel Mack,
Sebastian Frias
In-Reply-To: <7b10d4f3-68be-f5d7-633b-21854532ac26@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 11/04/2016 08:22 AM, Måns Rullgård wrote:
>> Andrew Lunn <andrew@lunn.ch> writes:
>>
>>> On Fri, Nov 04, 2016 at 03:05:00PM +0000, Måns Rullgård wrote:
>>>> Andrew Lunn <andrew@lunn.ch> writes:
>>>>
>>>>>>> I agree with you. But fixing it is likely to break boards which
>>>>>>> currently have "rgmii", but actually need the delay in order to work.
>>>>>>
>>>>>> Does the internal delay here refer to the PHY or the MAC? It's a
>>>>>> property of the MAC node after all.
>>>>>
>>>>> It is the PHY which applies the delay.
>>>>
>>>> Says who?
>>>
>>> The source code.
>>
>> There's source code that disagrees with that. The Broadcom GENET
>> driver, for instance.
>
> Correct, and in the case where the MAC adds the delay while transmitting
> (because it supports that) the expectation is that the PHY would remove
> such a delay internally, conversely, the PHY would introduce a delay
> while transmitting back to the PHY, in order to produce the desired 90
> degrees shift on the RGMII signals, and get reproduce the correct clock
> and data alignment internally.
>
>>
>>>> Some MACs can do it too.
>>>
>>> I'm sure they can. But look at the code. Nearly none do, and those
>>> that do are potentially broken.
>>
>> Those few drivers that do anything differently based on these values
>> enable clock delay in the MAC. That's why I wrote the NB8800 driver the
>> way I did.
>>
>
> I don't really what is wrong with the nb8800 driver at the moment, so
> maybe this is just a configuration issue with the Atheros PHY driver,
> it's not like it has not given people headache judging by the recent
> discussions...
We don't even know if the problems Mason is having are caused by
incorrect clock skew in the first place. I'd suggest not patching
anything at all until he gets it working.
--
Måns Rullgård
^ permalink raw reply
* [PATCH v2 2/2 ] net: ethernet: nb8800: handle all RGMII definitions
From: Sebastian Frias @ 2016-11-04 17:02 UTC (permalink / raw)
To: Florian Fainelli, Måns Rullgård, David S. Miller,
netdev
Cc: LKML, Mason, Andrew Lunn
In-Reply-To: <8c491fff-4849-fa9b-0a0a-5a7a6870120d@gmail.com>
Commit a999589ccaae ("phylib: add RGMII-ID interface mode definition")
and commit 7d400a4c5897 ("phylib: add PHY interface modes for internal
delay for tx and rx only") added several RGMII definitions:
PHY_INTERFACE_MODE_RGMII_ID, PHY_INTERFACE_MODE_RGMII_RXID and
PHY_INTERFACE_MODE_RGMII_TXID to deal with internal delays.
Those are all RGMII modes (1Gbit) and must be considered that way when
setting the MAC mode or the pad mode for the HW to work properly.
Signed-off-by: Sebastian Frias <sf84@laposte.net>
---
drivers/net/ethernet/aurora/nb8800.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index d2855c9..fba2699 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -598,6 +598,7 @@ static irqreturn_t nb8800_irq(int irq, void *dev_id)
static void nb8800_mac_config(struct net_device *dev)
{
struct nb8800_priv *priv = netdev_priv(dev);
+ struct phy_device *phydev = dev->phydev;
bool gigabit = priv->speed == SPEED_1000;
u32 mac_mode_mask = RGMII_MODE | HALF_DUPLEX | GMAC_MODE;
u32 mac_mode = 0;
@@ -609,7 +610,7 @@ static void nb8800_mac_config(struct net_device *dev)
mac_mode |= HALF_DUPLEX;
if (gigabit) {
- if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII)
+ if (phy_interface_is_rgmii(phydev))
mac_mode |= RGMII_MODE;
mac_mode |= GMAC_MODE;
@@ -1278,9 +1279,8 @@ static int nb8800_tangox_init(struct net_device *dev)
break;
case PHY_INTERFACE_MODE_RGMII:
- pad_mode = PAD_MODE_RGMII;
- break;
-
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
case PHY_INTERFACE_MODE_RGMII_TXID:
pad_mode = PAD_MODE_RGMII;
break;
--
1.7.11.2
^ permalink raw reply related
* [PATCH net-next 1/2] RDS: TCP: report addr/port info based on TCP socket in rds-info
From: Sowmini Varadhan @ 2016-11-04 17:04 UTC (permalink / raw)
To: netdev; +Cc: davem, rds-devel, sowmini.varadhan, santosh.shilimkar
In-Reply-To: <cover.1478276221.git.sowmini.varadhan@oracle.com>
The socket argument passed to rds_tcp_tc_info() is a PF_RDS socket,
so it is incorrect to report the address port info based on
rds_getname() as part of TCP state report.
Invoke inet_getname() for the t_sock associated with the
rds_tcp_connection instead.
Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
---
net/rds/tcp.c | 20 +++++++++++++-------
1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/net/rds/tcp.c b/net/rds/tcp.c
index fcddacc..3296a6a 100644
--- a/net/rds/tcp.c
+++ b/net/rds/tcp.c
@@ -220,7 +220,7 @@ void rds_tcp_set_callbacks(struct socket *sock, struct rds_conn_path *cp)
write_unlock_bh(&sock->sk->sk_callback_lock);
}
-static void rds_tcp_tc_info(struct socket *sock, unsigned int len,
+static void rds_tcp_tc_info(struct socket *rds_sock, unsigned int len,
struct rds_info_iterator *iter,
struct rds_info_lengths *lens)
{
@@ -229,6 +229,7 @@ static void rds_tcp_tc_info(struct socket *sock, unsigned int len,
unsigned long flags;
struct sockaddr_in sin;
int sinlen;
+ struct socket *sock;
spin_lock_irqsave(&rds_tcp_tc_list_lock, flags);
@@ -237,12 +238,17 @@ static void rds_tcp_tc_info(struct socket *sock, unsigned int len,
list_for_each_entry(tc, &rds_tcp_tc_list, t_list_item) {
- sock->ops->getname(sock, (struct sockaddr *)&sin, &sinlen, 0);
- tsinfo.local_addr = sin.sin_addr.s_addr;
- tsinfo.local_port = sin.sin_port;
- sock->ops->getname(sock, (struct sockaddr *)&sin, &sinlen, 1);
- tsinfo.peer_addr = sin.sin_addr.s_addr;
- tsinfo.peer_port = sin.sin_port;
+ sock = tc->t_sock;
+ if (sock) {
+ sock->ops->getname(sock, (struct sockaddr *)&sin,
+ &sinlen, 0);
+ tsinfo.local_addr = sin.sin_addr.s_addr;
+ tsinfo.local_port = sin.sin_port;
+ sock->ops->getname(sock, (struct sockaddr *)&sin,
+ &sinlen, 1);
+ tsinfo.peer_addr = sin.sin_addr.s_addr;
+ tsinfo.peer_port = sin.sin_port;
+ }
tsinfo.hdr_rem = tc->t_tinc_hdr_rem;
tsinfo.data_rem = tc->t_tinc_data_rem;
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 0/2] RDS: TCP: bug fixes
From: Sowmini Varadhan @ 2016-11-04 17:04 UTC (permalink / raw)
To: netdev; +Cc: davem, rds-devel, sowmini.varadhan, santosh.shilimkar
A couple of bug fixes identified during testing.
Sowmini Varadhan (2):
RDS: TCP: report addr/port info based on TCP socket in rds-info
RDS: TCP: start multipath acceptor loop at 0
net/rds/tcp.c | 20 +++++++++++++-------
net/rds/tcp_listen.c | 2 +-
2 files changed, 14 insertions(+), 8 deletions(-)
^ permalink raw reply
* [PATCH net-next 2/2] RDS: TCP: start multipath acceptor loop at 0
From: Sowmini Varadhan @ 2016-11-04 17:04 UTC (permalink / raw)
To: netdev; +Cc: davem, rds-devel, sowmini.varadhan, santosh.shilimkar
In-Reply-To: <cover.1478276221.git.sowmini.varadhan@oracle.com>
The for() loop in rds_tcp_accept_one() assumes that the 0'th
rds_tcp_conn_path is UP and starts multipath accepts at index 1.
But this assumption may not always be true: if the 0'th path
has failed (ERROR or DOWN state) an incoming connection request
should be used to resurrect this path.
Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
---
net/rds/tcp_listen.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
index e0b23fb..c9c4968 100644
--- a/net/rds/tcp_listen.c
+++ b/net/rds/tcp_listen.c
@@ -103,7 +103,7 @@ struct rds_tcp_connection *rds_tcp_accept_one_path(struct rds_connection *conn)
if (!peer_is_smaller)
return NULL;
- for (i = 1; i < npaths; i++) {
+ for (i = 0; i < npaths; i++) {
struct rds_conn_path *cp = &conn->c_path[i];
if (rds_conn_path_transition(cp, RDS_CONN_DOWN,
--
1.7.1
^ permalink raw reply related
* Re: Coding Style: Reverse XMAS tree declarations ? (was Re: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet)
From: Randy Dunlap @ 2016-11-04 17:05 UTC (permalink / raw)
To: Joe Perches, David Miller, madalin.bucur, Andrew Morton,
Jonathan Corbet
Cc: netdev, linuxppc-dev, linux-kernel, oss, ppc, pebolle,
joakim.tjernlund
In-Reply-To: <1478242438.1924.31.camel@perches.com>
On 11/03/16 23:53, Joe Perches wrote:
> On Thu, 2016-11-03 at 15:58 -0400, David Miller wrote:
>> From: Madalin Bucur <madalin.bucur@nxp.com>
>> Date: Wed, 2 Nov 2016 22:17:26 +0200
>>
>>> This introduces the Freescale Data Path Acceleration Architecture
>>> +static inline size_t bpool_buffer_raw_size(u8 index, u8 cnt)
>>> +{
>>> + u8 i;
>>> + size_t res = DPAA_BP_RAW_SIZE / 2;
>>
>> Always order local variable declarations from longest to shortest line,
>> also know as Reverse Christmas Tree Format.
>
> I think this declaration sorting order is misguided but
> here's a possible change to checkpatch adding a test for it
> that does this test just for net/ and drivers/net/
I agree with the misguided part.
That's not actually in CodingStyle AFAICT. Where did this come from?
thanks.
--
~Randy
^ permalink raw reply
* Re: Ethernet not working on a different SoC with same eth HW
From: Mason @ 2016-11-04 17:06 UTC (permalink / raw)
To: Mans Rullgard, Florian Fainelli
Cc: Andrew Lunn, netdev, Timur Tabi, Sergei Shtylyov, Zefir Kurtisi,
Martin Blumenstingl, Uwe Kleine-Konig, Daniel Mack,
Sebastian Frias
In-Reply-To: <yw1xwpgji3rd.fsf@unicorn.mansr.com>
On 04/11/2016 17:55, Måns Rullgård wrote:
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
>> On 11/04/2016 08:22 AM, Måns Rullgård wrote:
>>> Andrew Lunn <andrew@lunn.ch> writes:
>>>
>>>> On Fri, Nov 04, 2016 at 03:05:00PM +0000, Måns Rullgård wrote:
>>>>> Andrew Lunn <andrew@lunn.ch> writes:
>>>>>
>>>>>>>> I agree with you. But fixing it is likely to break boards which
>>>>>>>> currently have "rgmii", but actually need the delay in order to work.
>>>>>>>
>>>>>>> Does the internal delay here refer to the PHY or the MAC? It's a
>>>>>>> property of the MAC node after all.
>>>>>>
>>>>>> It is the PHY which applies the delay.
>>>>>
>>>>> Says who?
>>>>
>>>> The source code.
>>>
>>> There's source code that disagrees with that. The Broadcom GENET
>>> driver, for instance.
>>
>> Correct, and in the case where the MAC adds the delay while transmitting
>> (because it supports that) the expectation is that the PHY would remove
>> such a delay internally, conversely, the PHY would introduce a delay
>> while transmitting back to the PHY, in order to produce the desired 90
>> degrees shift on the RGMII signals, and get reproduce the correct clock
>> and data alignment internally.
>>
>>>
>>>>> Some MACs can do it too.
>>>>
>>>> I'm sure they can. But look at the code. Nearly none do, and those
>>>> that do are potentially broken.
>>>
>>> Those few drivers that do anything differently based on these values
>>> enable clock delay in the MAC. That's why I wrote the NB8800 driver the
>>> way I did.
>>>
>>
>> I don't really what is wrong with the nb8800 driver at the moment, so
>> maybe this is just a configuration issue with the Atheros PHY driver,
>> it's not like it has not given people headache judging by the recent
>> discussions...
>
> We don't even know if the problems Mason is having are caused by
> incorrect clock skew in the first place. I'd suggest not patching
> anything at all until he gets it working.
All I said was:
> Assuming that "rxid" (rx internal delay) and "rx clock delay" are
> in fact the same concept with different names, do you agree that
> it would be unexpected for "rgmii rx clock delay" to be enabled
> when a DTB specifies "rgmii" or "rgmii-txid" ?
In parallel, Sebastian changed the DT of a 8758 board
from phy-connection-type = "rgmii";
to phy-connection-type = "rgmii-txid";
and this broke the Ethernet on 8758, although the "reference"
legacy 3.4 kernel does enable tx clock delay.
Regards.
^ permalink raw reply
* Re: [PATCH v2 2/2 ] net: ethernet: nb8800: handle all RGMII definitions
From: Måns Rullgård @ 2016-11-04 17:12 UTC (permalink / raw)
To: Sebastian Frias
Cc: Florian Fainelli, David S. Miller, netdev, LKML, Mason,
Andrew Lunn
In-Reply-To: <581CBF17.2010108@laposte.net>
Sebastian Frias <sf84@laposte.net> writes:
> Commit a999589ccaae ("phylib: add RGMII-ID interface mode definition")
> and commit 7d400a4c5897 ("phylib: add PHY interface modes for internal
> delay for tx and rx only") added several RGMII definitions:
> PHY_INTERFACE_MODE_RGMII_ID, PHY_INTERFACE_MODE_RGMII_RXID and
> PHY_INTERFACE_MODE_RGMII_TXID to deal with internal delays.
>
> Those are all RGMII modes (1Gbit) and must be considered that way when
> setting the MAC mode or the pad mode for the HW to work properly.
>
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
> ---
> drivers/net/ethernet/aurora/nb8800.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
> index d2855c9..fba2699 100644
> --- a/drivers/net/ethernet/aurora/nb8800.c
> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -598,6 +598,7 @@ static irqreturn_t nb8800_irq(int irq, void *dev_id)
> static void nb8800_mac_config(struct net_device *dev)
> {
> struct nb8800_priv *priv = netdev_priv(dev);
> + struct phy_device *phydev = dev->phydev;
> bool gigabit = priv->speed == SPEED_1000;
> u32 mac_mode_mask = RGMII_MODE | HALF_DUPLEX | GMAC_MODE;
> u32 mac_mode = 0;
> @@ -609,7 +610,7 @@ static void nb8800_mac_config(struct net_device *dev)
> mac_mode |= HALF_DUPLEX;
>
> if (gigabit) {
> - if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII)
> + if (phy_interface_is_rgmii(phydev))
> mac_mode |= RGMII_MODE;
>
> mac_mode |= GMAC_MODE;
This part is correct regardless of the outcome of the delay setup
discussion.
> @@ -1278,9 +1279,8 @@ static int nb8800_tangox_init(struct net_device *dev)
> break;
>
> case PHY_INTERFACE_MODE_RGMII:
> - pad_mode = PAD_MODE_RGMII;
> - break;
> -
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + case PHY_INTERFACE_MODE_RGMII_RXID:
> case PHY_INTERFACE_MODE_RGMII_TXID:
> pad_mode = PAD_MODE_RGMII;
> break;
> --
> 1.7.11.2
>
--
Måns Rullgård
^ permalink raw reply
* Re: Coding Style: Reverse XMAS tree declarations ?
From: Joe Perches @ 2016-11-04 17:44 UTC (permalink / raw)
To: David Miller, lsanfil
Cc: madalin.bucur, akpm, corbet, netdev, linuxppc-dev, linux-kernel,
oss, ppc, pebolle, joakim.tjernlund, Randy Dunlap
In-Reply-To: <20161104.110759.1093635654135143910.davem@davemloft.net>
On Fri, 2016-11-04 at 11:07 -0400, David Miller wrote:
> From: Lino Sanfilippo <lsanfil@marvell.com>
> > On 04.11.2016 07:53, Joe Perches wrote:
> >> CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to
> >> shortest
> >> #446: FILE: drivers/net/ethernet/ethoc.c:446:
> >> + int size = bd.stat >> 16;
> >> + struct sk_buff *skb;
> > should not this case be valid? Optically the longer line is already
> > before the shorter.
> > I think that the whole point in using this reverse xmas tree ordering
> > is to have
> > the code optically tidied up and not to enforce ordering between
> > variable name lengths.
>
> That's correct.
And also another reason the whole reverse xmas tree
automatic declaration layout concept is IMO dubious.
Basically, you're looking not at the initial ordering
of automatics as important, but helping find a specific
automatic when reversing from reading code is not always
correct.
Something like:
static void function{args,...)
{
[longish list of reverse xmas tree identifiers...]
struct foo *bar = longish_function(args, ...);
struct foobarbaz *qux;
[more identifers]
[multiple screenfuls of code later...)
new_function(..., bar, ...);
[more code...]
}
and the reverse xmas tree helpfulness of looking up the
type of bar is neither obvious nor easy.
My preference would be for a bar that serves coffee and alcohol.
^ permalink raw reply
* Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
From: 张谦 @ 2016-11-04 17:45 UTC (permalink / raw)
To: Jon Maloy
Cc: Ben Hutchings, Ying Xue, netdev@vger.kernel.org, Eric Dumazet,
tyrande000@gmail.com
In-Reply-To: <A2BAEFC30C8FD34388F02C9B3121859D2259F9B9@eusaamb103.ericsson.se>
yes, tipc_l2_device_event() only change MTU of bearer rather than the MTU of link, tipc_enable_l2_media() will be the right place to test a tiny MTU.
Qian
Sent from my iPhone
On 5 Nov 2016, at 00:00, Jon Maloy <jon.maloy@ericsson.com> wrote:
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>> On Behalf Of ??
>> Sent: Friday, 04 November, 2016 03:24
>> To: Jon Maloy <jon.maloy@ericsson.com>; Ben Hutchings
>> <ben@decadent.org.uk>; Ying Xue <ying.xue0@gmail.com>
>> Cc: netdev@vger.kernel.org; Eric Dumazet <edumazet@google.com>
>> Subject: Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
>>
>> Hi,
>> I think both tipc_l2_device_event() and tipc_enable_l2_media() need to refuse a
>> tiny MTU for TIPC bearers.
>
> Right, except that when looking into the code for tipc_l2_device_event() I realize that it currently doesn't try to re-adapt to a new MTU at all. It just calls tipc_reset_bearer(), which I suspect has changed somewhere along the road to ignore the MTU. So, you only need to change tipc_enable_l2_media().
>
> ///jon
>
>>
>> tipc_l2_device_event() used to update the TIPC MTU value when executing a
>> command like 'ifconfig eth0 MTU 1 up'.
>> tipc_enable_l2_media() will be invoked when the TIPC network created.
>>
>> Thanks.
>>
>> Qian Zhang
>> MarvelTeam Qihoo 360
>>
>>
>>
>> -----邮件原件-----
>> 发件人: Jon Maloy [mailto:jon.maloy@ericsson.com]
>> 发送时间: 2016年11月1日 19:37
>> 收件人: 张谦; Ben Hutchings; Ying Xue
>> 抄送: netdev@vger.kernel.org; Eric Dumazet
>> 主题: RE: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
>>
>> Hi,
>> I think we all agreed in the end that this is a possible, but highly implausible,
>> scenario, and rather as a point of exploit than a functional bug.
>> The solution is very simple, and described further down in this mail thread. I have
>> not done anything to it yet, but you are welcome to contribute.
>>
>> BR
>> ///jon
>>
>>
>>> -----Original Message-----
>>> From: 张谦 [mailto:zhangqian-c@360.cn]
>>> Sent: Tuesday, 01 November, 2016 02:35
>>> To: Ben Hutchings <ben@decadent.org.uk>; Jon Maloy
>>> <jon.maloy@ericsson.com>; Ying Xue <ying.xue0@gmail.com>
>>> Cc: netdev@vger.kernel.org; Eric Dumazet <edumazet@google.com>
>>> Subject: Re: [PATCH net] tipc: Guard against tiny MTU in
>>> tipc_msg_build()
>>>
>>> Hi all,
>>> I have accomplished a PoC can help you to confirm this issue.
>>>
>>> And two weeks passed from the last mail, can you tell me the progress
>>> of the patch to this flaw?
>>>
>>> Thanks.
>>>
>>> Qian Zhang
>>> Marvel Team Qihoo 360
>>>
>>>
>>> -----邮件原件-----
>>> 发件人: Ben Hutchings [mailto:ben@decadent.org.uk]
>>> 发送时间: 2016年10月21日 23:00
>>> 收件人: Jon Maloy; Ying Xue
>>> 抄送: netdev@vger.kernel.org; 张谦; Eric Dumazet
>>> 主题: Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
>>>
>>> On Fri, 2016-10-21 at 14:57 +0000, Jon Maloy wrote:
>>>>> -----Original Message-----
>>>>>>> From: Ben Hutchings [mailto:ben@decadent.org.uk]
>>>>> Sent: Thursday, 20 October, 2016 12:40
>>>>>>> To: Jon Maloy <jon.maloy@ericsson.com>; Ying Xue
>>>>>>> <ying.xue0@gmail.com>
>>>>>>>>> Cc: netdev@vger.kernel.org; Qian Zhang
>>>>>>>>> <zhangqian-c@360.cn>; Eric Dumazet
>>>>>>> <edumazet@google.com>
>>>>> Subject: Re: [PATCH net] tipc: Guard against tiny MTU in
>>>>> tipc_msg_build()
>>>>>
>>>>> On Thu, 2016-10-20 at 14:51 +0000, Jon Maloy wrote:
>>>>> [...]
>>>>>>> At this point we're about to copy INT_H_SIZE + mhsz bytes into
>>>>>>> the first fragment. If that's already limited to be less than
>>>>>>> or equal to MAX_H_SIZE, comparing with MAX_H_SIZE would be fine.
>>>>>>> But if
>>>>>
>>>>> MAX_H_SIZE
>>>>>>> is the maximum value of mhsz, that won't be good enough.
>>>>>>
>>>>>>
>>>>>>
>>>>>> MAX_H_SIZE is 60 bytes, but in practice you will never see an
>>>>>> mhsz larger than
>>>>>
>>>>> the biggest header we are actually using, which is MCAST_H_SIZE
>>>>> (==44
>>> bytes).
>>>>>> INT_H_SIZE is 40 bytes, so you are in reality testing for
>>>>>> whether we have an mtu
>>>>>
>>>>> < 84 bytes.
>>>>>> You won't find any interfaces or protocols that come even close
>>>>>> to this
>>>>>
>>>>> limitation, so to me this test is redundant.
>>>>>
>>>>> But I can easily create such an interface:
>>>>>
>>>>> $ unshare -n -U -r
>>>>> # ip l set lo mtu 1
>>>>>
>>>>> Ben.
>>>>
>>>>
>>>> It won't be very useful though. But I assume you mean it could be a
>>>> possible exploit,
>>>
>>> Exactly.
>>>
>>>> and I suspect a few other things would break both in TIPC and in
>>>> other stacks if you do anything like that. I think the solution to
>>>> this is not to fix all possible places in the code where this can go
>>>> wrong, but rather to have a generic test where we refuse to attach
>>>> bearers/interfaces offering an mtu < e.g. 1000 bytes. This can
>>>> easily be done in tipc_enable_l2_media().
>>>
>>> Yes.
>>>
>>> Ben.
>>>
>>> --
>>> Ben Hutchings
>>> One of the nice things about standards is that there are so many of them.
>
^ permalink raw reply
* Re: [PATCH v2 2/2 ] net: ethernet: nb8800: handle all RGMII definitions
From: David Miller @ 2016-11-04 17:54 UTC (permalink / raw)
To: sf84; +Cc: f.fainelli, mans, netdev, linux-kernel, slash.tmp, andrew
In-Reply-To: <581CBF17.2010108@laposte.net>
From: Sebastian Frias <sf84@laposte.net>
Date: Fri, 4 Nov 2016 18:02:15 +0100
> Commit a999589ccaae ("phylib: add RGMII-ID interface mode definition")
> and commit 7d400a4c5897 ("phylib: add PHY interface modes for internal
> delay for tx and rx only") added several RGMII definitions:
> PHY_INTERFACE_MODE_RGMII_ID, PHY_INTERFACE_MODE_RGMII_RXID and
> PHY_INTERFACE_MODE_RGMII_TXID to deal with internal delays.
>
> Those are all RGMII modes (1Gbit) and must be considered that way when
> setting the MAC mode or the pad mode for the HW to work properly.
>
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
You cannot just repost one part of a patch series when you make changes.
You must always repost the entire series as a new fresh version, with
changelog entries added to your "[PATCH v2 0/2] ..." header posting.
^ permalink raw reply
* [net-next v2 0/7] stmmac: dwmac-sti refactor+cleanup
From: Joachim Eastwood @ 2016-11-04 17:54 UTC (permalink / raw)
To: davem; +Cc: Joachim Eastwood, peppe.cavallaro, alexandre.torgue, netdev
This patch set aims to remove the init/exit callbacks from the
dwmac-sti driver and instead use standard PM callbacks. Doing this
will also allow us to cleanup the driver.
Eventually the init/exit callbacks will be deprecated and removed
from all drivers dwmac-* except for dwmac-generic. Drivers will be
refactored to use standard PM and remove callbacks.
Changes since v2:
- add missing static to sti_dwmac_pm_ops
- s/sti_dwmac_set_phy_mode/sti_dwmac_set_mode
- acked/tested by Giuseppe
Joachim Eastwood (7):
stmmac: dwmac-sti: remove useless of_node check
stmmac: dwmac-sti: remove clk NULL checks
stmmac: dwmac-sti: add PM ops and resume function
stmmac: dwmac-sti: move st,gmac_en parsing to sti_dwmac_parse_data
stmmac: dwmac-sti: move clk_prepare_enable out of init and add error handling
stmmac: dwmac-sti: clean up and rename sti_dwmac_init
stmmac: dwmac-sti: remove unused priv dev member
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 87 ++++++++++++++++---------
1 file changed, 58 insertions(+), 29 deletions(-)
--
2.10.2
^ permalink raw reply
* [net-next v2 1/7] stmmac: dwmac-sti: remove useless of_node check
From: Joachim Eastwood @ 2016-11-04 17:54 UTC (permalink / raw)
To: davem; +Cc: Joachim Eastwood, peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <20161104175411.12943-1-manabian@gmail.com>
Since dwmac-sti is a DT only driver checking for OF node is not necessary.
Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Tested-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
index 58c05ac..075ed42 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
@@ -270,9 +270,6 @@ static int sti_dwmac_parse_data(struct sti_dwmac *dwmac,
struct regmap *regmap;
int err;
- if (!np)
- return -EINVAL;
-
/* clk selection from extra syscfg register */
dwmac->clk_sel_reg = -ENXIO;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sti-clkconf");
--
2.10.2
^ permalink raw reply related
* [net-next v2 2/7] stmmac: dwmac-sti: remove clk NULL checks
From: Joachim Eastwood @ 2016-11-04 17:54 UTC (permalink / raw)
To: davem; +Cc: Joachim Eastwood, peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <20161104175411.12943-1-manabian@gmail.com>
Since sti_dwmac_parse_data() sets dwmac->clk to NULL if not clock was
provided in DT and NULL is a valid clock there is no need to check for
NULL before using this clock.
Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Tested-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
index 075ed42..f009bf4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
@@ -191,7 +191,7 @@ static void stih4xx_fix_retime_src(void *priv, u32 spd)
}
}
- if (src == TX_RETIME_SRC_CLKGEN && dwmac->clk && freq)
+ if (src == TX_RETIME_SRC_CLKGEN && freq)
clk_set_rate(dwmac->clk, freq);
regmap_update_bits(dwmac->regmap, reg, STIH4XX_RETIME_SRC_MASK,
@@ -222,7 +222,7 @@ static void stid127_fix_retime_src(void *priv, u32 spd)
freq = DWMAC_2_5MHZ;
}
- if (dwmac->clk && freq)
+ if (freq)
clk_set_rate(dwmac->clk, freq);
regmap_update_bits(dwmac->regmap, reg, STID127_RETIME_SRC_MASK, val);
@@ -238,8 +238,7 @@ static int sti_dwmac_init(struct platform_device *pdev, void *priv)
u32 reg = dwmac->ctrl_reg;
u32 val;
- if (dwmac->clk)
- clk_prepare_enable(dwmac->clk);
+ clk_prepare_enable(dwmac->clk);
if (of_property_read_bool(np, "st,gmac_en"))
regmap_update_bits(regmap, reg, EN_MASK, EN);
@@ -258,8 +257,7 @@ static void sti_dwmac_exit(struct platform_device *pdev, void *priv)
{
struct sti_dwmac *dwmac = priv;
- if (dwmac->clk)
- clk_disable_unprepare(dwmac->clk);
+ clk_disable_unprepare(dwmac->clk);
}
static int sti_dwmac_parse_data(struct sti_dwmac *dwmac,
struct platform_device *pdev)
--
2.10.2
^ permalink raw reply related
* [net-next v2 3/7] stmmac: dwmac-sti: add PM ops and resume function
From: Joachim Eastwood @ 2016-11-04 17:54 UTC (permalink / raw)
To: davem; +Cc: Joachim Eastwood, peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <20161104175411.12943-1-manabian@gmail.com>
Implement PM callbacks and driver remove in the driver instead
of relying on the init/exit hooks in stmmac_platform. This gives
the driver more flexibility in how the code is organized.
Eventually the init/exit callbacks will be deprecated in favor
of the standard PM callbacks and driver remove function.
Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Tested-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 47 +++++++++++++++++++------
1 file changed, 37 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
index f009bf4..bd6db22 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
@@ -253,12 +253,6 @@ static int sti_dwmac_init(struct platform_device *pdev, void *priv)
return 0;
}
-static void sti_dwmac_exit(struct platform_device *pdev, void *priv)
-{
- struct sti_dwmac *dwmac = priv;
-
- clk_disable_unprepare(dwmac->clk);
-}
static int sti_dwmac_parse_data(struct sti_dwmac *dwmac,
struct platform_device *pdev)
{
@@ -352,8 +346,6 @@ static int sti_dwmac_probe(struct platform_device *pdev)
dwmac->fix_retime_src = data->fix_retime_src;
plat_dat->bsp_priv = dwmac;
- plat_dat->init = sti_dwmac_init;
- plat_dat->exit = sti_dwmac_exit;
plat_dat->fix_mac_speed = data->fix_retime_src;
ret = sti_dwmac_init(pdev, plat_dat->bsp_priv);
@@ -363,6 +355,41 @@ static int sti_dwmac_probe(struct platform_device *pdev)
return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
}
+static int sti_dwmac_remove(struct platform_device *pdev)
+{
+ struct sti_dwmac *dwmac = get_stmmac_bsp_priv(&pdev->dev);
+ int ret = stmmac_dvr_remove(&pdev->dev);
+
+ clk_disable_unprepare(dwmac->clk);
+
+ return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int sti_dwmac_suspend(struct device *dev)
+{
+ struct sti_dwmac *dwmac = get_stmmac_bsp_priv(dev);
+ int ret = stmmac_suspend(dev);
+
+ clk_disable_unprepare(dwmac->clk);
+
+ return ret;
+}
+
+static int sti_dwmac_resume(struct device *dev)
+{
+ struct sti_dwmac *dwmac = get_stmmac_bsp_priv(dev);
+ struct platform_device *pdev = to_platform_device(dev);
+
+ sti_dwmac_init(pdev, dwmac);
+
+ return stmmac_resume(dev);
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(sti_dwmac_pm_ops, sti_dwmac_suspend,
+ sti_dwmac_resume);
+
static const struct sti_dwmac_of_data stih4xx_dwmac_data = {
.fix_retime_src = stih4xx_fix_retime_src,
};
@@ -382,10 +409,10 @@ MODULE_DEVICE_TABLE(of, sti_dwmac_match);
static struct platform_driver sti_dwmac_driver = {
.probe = sti_dwmac_probe,
- .remove = stmmac_pltfr_remove,
+ .remove = sti_dwmac_remove,
.driver = {
.name = "sti-dwmac",
- .pm = &stmmac_pltfr_pm_ops,
+ .pm = &sti_dwmac_pm_ops,
.of_match_table = sti_dwmac_match,
},
};
--
2.10.2
^ permalink raw reply related
* [net-next v2 4/7] stmmac: dwmac-sti: move st,gmac_en parsing to sti_dwmac_parse_data
From: Joachim Eastwood @ 2016-11-04 17:54 UTC (permalink / raw)
To: davem; +Cc: Joachim Eastwood, peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <20161104175411.12943-1-manabian@gmail.com>
The sti_dwmac_init() function is called both from probe and resume.
Since DT properties doesn't change between suspend/resume cycles move
parsing of this parameter into sti_dwmac_parse_data() where it belongs.
Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Tested-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
index bd6db22..269c7a5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
@@ -128,6 +128,7 @@ struct sti_dwmac {
int clk_sel_reg; /* GMAC ext clk selection register */
struct device *dev;
struct regmap *regmap;
+ bool gmac_en;
u32 speed;
void (*fix_retime_src)(void *priv, unsigned int speed);
};
@@ -233,14 +234,12 @@ static int sti_dwmac_init(struct platform_device *pdev, void *priv)
struct sti_dwmac *dwmac = priv;
struct regmap *regmap = dwmac->regmap;
int iface = dwmac->interface;
- struct device *dev = dwmac->dev;
- struct device_node *np = dev->of_node;
u32 reg = dwmac->ctrl_reg;
u32 val;
clk_prepare_enable(dwmac->clk);
- if (of_property_read_bool(np, "st,gmac_en"))
+ if (dwmac->gmac_en)
regmap_update_bits(regmap, reg, EN_MASK, EN);
regmap_update_bits(regmap, reg, MII_PHY_SEL_MASK, phy_intf_sels[iface]);
@@ -281,6 +280,7 @@ static int sti_dwmac_parse_data(struct sti_dwmac *dwmac,
dwmac->dev = dev;
dwmac->interface = of_get_phy_mode(np);
dwmac->regmap = regmap;
+ dwmac->gmac_en = of_property_read_bool(np, "st,gmac_en");
dwmac->ext_phyclk = of_property_read_bool(np, "st,ext-phyclk");
dwmac->tx_retime_src = TX_RETIME_SRC_NA;
dwmac->speed = SPEED_100;
--
2.10.2
^ permalink raw reply related
* [net-next v2 5/7] stmmac: dwmac-sti: move clk_prepare_enable out of init and add error handling
From: Joachim Eastwood @ 2016-11-04 17:54 UTC (permalink / raw)
To: davem; +Cc: Joachim Eastwood, peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <20161104175411.12943-1-manabian@gmail.com>
Add clock error handling to probe and in the process move clock enabling
out of sti_dwmac_init() to make this easier.
Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Tested-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
index 269c7a5..b71888a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
@@ -237,8 +237,6 @@ static int sti_dwmac_init(struct platform_device *pdev, void *priv)
u32 reg = dwmac->ctrl_reg;
u32 val;
- clk_prepare_enable(dwmac->clk);
-
if (dwmac->gmac_en)
regmap_update_bits(regmap, reg, EN_MASK, EN);
@@ -348,11 +346,23 @@ static int sti_dwmac_probe(struct platform_device *pdev)
plat_dat->bsp_priv = dwmac;
plat_dat->fix_mac_speed = data->fix_retime_src;
- ret = sti_dwmac_init(pdev, plat_dat->bsp_priv);
+ ret = clk_prepare_enable(dwmac->clk);
if (ret)
return ret;
- return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+ ret = sti_dwmac_init(pdev, plat_dat->bsp_priv);
+ if (ret)
+ goto disable_clk;
+
+ ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+ if (ret)
+ goto disable_clk;
+
+ return 0;
+
+disable_clk:
+ clk_disable_unprepare(dwmac->clk);
+ return ret;
}
static int sti_dwmac_remove(struct platform_device *pdev)
@@ -381,6 +391,7 @@ static int sti_dwmac_resume(struct device *dev)
struct sti_dwmac *dwmac = get_stmmac_bsp_priv(dev);
struct platform_device *pdev = to_platform_device(dev);
+ clk_prepare_enable(dwmac->clk);
sti_dwmac_init(pdev, dwmac);
return stmmac_resume(dev);
--
2.10.2
^ permalink raw reply related
* [net-next v2 6/7] stmmac: dwmac-sti: clean up and rename sti_dwmac_init
From: Joachim Eastwood @ 2016-11-04 17:54 UTC (permalink / raw)
To: davem; +Cc: Joachim Eastwood, peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <20161104175411.12943-1-manabian@gmail.com>
Rename sti_dwmac_init to sti_dwmac_set_mode which is a better
description for what it really does.
Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Tested-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
index b71888a..ac17bff 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
@@ -229,9 +229,8 @@ static void stid127_fix_retime_src(void *priv, u32 spd)
regmap_update_bits(dwmac->regmap, reg, STID127_RETIME_SRC_MASK, val);
}
-static int sti_dwmac_init(struct platform_device *pdev, void *priv)
+static int sti_dwmac_set_mode(struct sti_dwmac *dwmac)
{
- struct sti_dwmac *dwmac = priv;
struct regmap *regmap = dwmac->regmap;
int iface = dwmac->interface;
u32 reg = dwmac->ctrl_reg;
@@ -245,7 +244,7 @@ static int sti_dwmac_init(struct platform_device *pdev, void *priv)
val = (iface == PHY_INTERFACE_MODE_REVMII) ? 0 : ENMII;
regmap_update_bits(regmap, reg, ENMII_MASK, val);
- dwmac->fix_retime_src(priv, dwmac->speed);
+ dwmac->fix_retime_src(dwmac, dwmac->speed);
return 0;
}
@@ -350,7 +349,7 @@ static int sti_dwmac_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = sti_dwmac_init(pdev, plat_dat->bsp_priv);
+ ret = sti_dwmac_set_mode(dwmac);
if (ret)
goto disable_clk;
@@ -389,10 +388,9 @@ static int sti_dwmac_suspend(struct device *dev)
static int sti_dwmac_resume(struct device *dev)
{
struct sti_dwmac *dwmac = get_stmmac_bsp_priv(dev);
- struct platform_device *pdev = to_platform_device(dev);
clk_prepare_enable(dwmac->clk);
- sti_dwmac_init(pdev, dwmac);
+ sti_dwmac_set_mode(dwmac);
return stmmac_resume(dev);
}
--
2.10.2
^ permalink raw reply related
* [net-next v2 7/7] stmmac: dwmac-sti: remove unused priv dev member
From: Joachim Eastwood @ 2016-11-04 17:54 UTC (permalink / raw)
To: davem; +Cc: Joachim Eastwood, peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <20161104175411.12943-1-manabian@gmail.com>
The dev member of struct sti_dwmac is not used anywhere in the driver
so lets just remove it.
Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Tested-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
index ac17bff..c9006ab 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
@@ -126,7 +126,6 @@ struct sti_dwmac {
struct clk *clk; /* PHY clock */
u32 ctrl_reg; /* GMAC glue-logic control register */
int clk_sel_reg; /* GMAC ext clk selection register */
- struct device *dev;
struct regmap *regmap;
bool gmac_en;
u32 speed;
@@ -274,7 +273,6 @@ static int sti_dwmac_parse_data(struct sti_dwmac *dwmac,
return err;
}
- dwmac->dev = dev;
dwmac->interface = of_get_phy_mode(np);
dwmac->regmap = regmap;
dwmac->gmac_en = of_property_read_bool(np, "st,gmac_en");
--
2.10.2
^ permalink raw reply related
* [PATCH v2 0/2] net: ethernet: nb8800: Do not apply TX delay at MAC level
From: Sebastian Frias @ 2016-11-04 18:17 UTC (permalink / raw)
To: Måns Rullgård, David S. Miller, netdev; +Cc: LKML, Mason, Andrew Lunn
This is v2 of the series, only the second patch:
"net: ethernet: nb8800: handle all RGMII definitions" is modified
to account for Florian's suggestion.
Sebastian Frias (2):
net: ethernet: nb8800: Do not apply TX delay at MAC level
net: ethernet: nb8800: handle all RGMII definitions
drivers/net/ethernet/aurora/nb8800.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
--
1.7.11.2
^ permalink raw reply
* [PATCH v2 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level
From: Sebastian Frias @ 2016-11-04 18:18 UTC (permalink / raw)
To: Måns Rullgård, David S. Miller, netdev; +Cc: LKML, Mason, Andrew Lunn
In-Reply-To: <581CD0CA.9070506@laposte.net>
The delay can be applied at PHY or MAC level, but since
PHY drivers will apply the delay at PHY level when using
one of the "internal delay" declinations of RGMII mode
(like PHY_INTERFACE_MODE_RGMII_TXID), applying it again
at MAC level causes issues.
Signed-off-by: Sebastian Frias <sf84@laposte.net>
---
drivers/net/ethernet/aurora/nb8800.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index b59aa35..d2855c9 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -1282,7 +1282,7 @@ static int nb8800_tangox_init(struct net_device *dev)
break;
case PHY_INTERFACE_MODE_RGMII_TXID:
- pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
+ pad_mode = PAD_MODE_RGMII;
break;
default:
--
1.7.11.2
^ permalink raw reply related
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