diff mbox

[4/9] net: ethernet: ti: cpts: move dt props parsing to cpts driver

Message ID 20160914130231.3035-5-grygorii.strashko@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Grygorii Strashko Sept. 14, 2016, 1:02 p.m. UTC
Move DT properties parsing into CPTS driver to simplify consumer's
code and CPTS driver porting on other SoC in the future
(like Keystone 2).

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 16 +---------------
 drivers/net/ethernet/ti/cpsw.h |  2 --
 drivers/net/ethernet/ti/cpts.c | 29 ++++++++++++++++++++++++++---
 drivers/net/ethernet/ti/cpts.h |  5 +++--
 4 files changed, 30 insertions(+), 22 deletions(-)

Comments

Richard Cochran Sept. 14, 2016, 1:55 p.m. UTC | #1
On Wed, Sep 14, 2016 at 04:02:26PM +0300, Grygorii Strashko wrote:
> Move DT properties parsing into CPTS driver to simplify consumer's
> code and CPTS driver porting on other SoC in the future
> (like Keystone 2).

And just who is the consumer?
 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  drivers/net/ethernet/ti/cpsw.c | 16 +---------------
>  drivers/net/ethernet/ti/cpsw.h |  2 --
>  drivers/net/ethernet/ti/cpts.c | 29 ++++++++++++++++++++++++++---
>  drivers/net/ethernet/ti/cpts.h |  5 +++--
>  4 files changed, 30 insertions(+), 22 deletions(-)

You have more (+) than (-).  I wouldn't call that a simplification.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grygorii Strashko Sept. 14, 2016, 7:45 p.m. UTC | #2
On 09/14/2016 04:55 PM, Richard Cochran wrote:
> On Wed, Sep 14, 2016 at 04:02:26PM +0300, Grygorii Strashko wrote:
>> Move DT properties parsing into CPTS driver to simplify consumer's
>> code and CPTS driver porting on other SoC in the future
>> (like Keystone 2).
> 
> And just who is the consumer?

Now CPSW, and Keystone 2 netcp in the future.

>  
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>  drivers/net/ethernet/ti/cpsw.c | 16 +---------------
>>  drivers/net/ethernet/ti/cpsw.h |  2 --
>>  drivers/net/ethernet/ti/cpts.c | 29 ++++++++++++++++++++++++++---
>>  drivers/net/ethernet/ti/cpts.h |  5 +++--
>>  4 files changed, 30 insertions(+), 22 deletions(-)
> 
> You have more (+) than (-).  I wouldn't call that a simplification.


With this change It will not be required to add the same DT parsing code
in Keystone 2 netcp driver, so overall number of lines will be reduced.

As I mentioned in cover letter - this is preparation step and this series
 indented help and simplify CPTS integration with few TI net drivers
(now cpsw, soon KS2 netcp), because similar CPTS IP is integrated
in two different TI's SoC families (and might be in more the future).  

I've not posted KS2 patches here, because of two reasons:
- they are still under internal review
- I hate big patch series
Richard Cochran Sept. 14, 2016, 8:03 p.m. UTC | #3
On Wed, Sep 14, 2016 at 10:45:54PM +0300, Grygorii Strashko wrote:
> With this change It will not be required to add the same DT parsing code
> in Keystone 2 netcp driver, so overall number of lines will be reduced.

This explanation would make the commit message much more informative.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index dfd5707..3db8fec 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2311,18 +2311,6 @@  static int cpsw_probe_dt(struct cpsw_platform_data *data,
 	}
 	data->active_slave = prop;
 
-	if (of_property_read_u32(node, "cpts_clock_mult", &prop)) {
-		dev_err(&pdev->dev, "Missing cpts_clock_mult property in the DT.\n");
-		return -EINVAL;
-	}
-	data->cpts_clock_mult = prop;
-
-	if (of_property_read_u32(node, "cpts_clock_shift", &prop)) {
-		dev_err(&pdev->dev, "Missing cpts_clock_shift property in the DT.\n");
-		return -EINVAL;
-	}
-	data->cpts_clock_shift = prop;
-
 	data->slave_data = devm_kzalloc(&pdev->dev, data->slaves
 					* sizeof(struct cpsw_slave_data),
 					GFP_KERNEL);
@@ -2742,9 +2730,7 @@  static int cpsw_probe(struct platform_device *pdev)
 		goto clean_dma_ret;
 	}
 
-	cpsw->cpts = cpts_create(cpsw->dev, cpts_regs,
-				 cpsw->data.cpts_clock_mult,
-				 cpsw->data.cpts_clock_shift);
+	cpsw->cpts = cpts_create(cpsw->dev, cpts_regs, cpsw->dev->of_node);
 	if (IS_ERR(cpsw->cpts)) {
 		ret = PTR_ERR(cpsw->cpts);
 		goto clean_ale_ret;
diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index 16b54c6..6c3037a 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -31,8 +31,6 @@  struct cpsw_platform_data {
 	u32	channels;	/* number of cpdma channels (symmetric) */
 	u32	slaves;		/* number of slave cpgmac ports */
 	u32	active_slave; /* time stamping, ethtool and SIOCGMIIPHY slave */
-	u32	cpts_clock_mult;  /* convert input clock ticks to nanoseconds */
-	u32	cpts_clock_shift; /* convert input clock ticks to nanoseconds */
 	u32	ale_entries;	/* ale table size */
 	u32	bd_ram_size;  /*buffer descriptor ram size */
 	u32	mac_control;	/* Mac control register */
diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index a46478e..1ee64c6 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -388,10 +388,31 @@  void cpts_unregister(struct cpts *cpts)
 	clk_disable(cpts->refclk);
 }
 
+static int cpts_of_parse(struct cpts *cpts, struct device_node *node)
+{
+	int ret = -EINVAL;
+	u32 prop;
+
+	if (of_property_read_u32(node, "cpts_clock_mult", &prop))
+		goto  of_error;
+	cpts->cc_mult = prop;
+
+	if (of_property_read_u32(node, "cpts_clock_shift", &prop))
+		goto  of_error;
+	cpts->cc.shift = prop;
+
+	return 0;
+
+of_error:
+	dev_err(cpts->dev, "CPTS: Missing property in the DT.\n");
+	return ret;
+}
+
 struct cpts *cpts_create(struct device *dev, void __iomem *regs,
-			 u32 mult, u32 shift)
+			 struct device_node *node)
 {
 	struct cpts *cpts;
+	int ret;
 
 	if (!regs || !dev)
 		return ERR_PTR(-EINVAL);
@@ -405,6 +426,10 @@  struct cpts *cpts_create(struct device *dev, void __iomem *regs,
 	spin_lock_init(&cpts->lock);
 	INIT_DELAYED_WORK(&cpts->overflow_work, cpts_overflow_check);
 
+	ret = cpts_of_parse(cpts, node);
+	if (ret)
+		return ERR_PTR(ret);
+
 	cpts->refclk = devm_clk_get(dev, "cpts");
 	if (IS_ERR(cpts->refclk)) {
 		dev_err(dev, "Failed to get cpts refclk\n");
@@ -415,8 +440,6 @@  struct cpts *cpts_create(struct device *dev, void __iomem *regs,
 
 	cpts->cc.read = cpts_systim_read;
 	cpts->cc.mask = CLOCKSOURCE_MASK(32);
-	cpts->cc.shift = shift;
-	cpts->cc_mult = mult;
 
 	return cpts;
 }
diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
index 0c02f48..a865193 100644
--- a/drivers/net/ethernet/ti/cpts.h
+++ b/drivers/net/ethernet/ti/cpts.h
@@ -27,6 +27,7 @@ 
 #include <linux/clocksource.h>
 #include <linux/device.h>
 #include <linux/list.h>
+#include <linux/of.h>
 #include <linux/ptp_clock_kernel.h>
 #include <linux/skbuff.h>
 #include <linux/timecounter.h>
@@ -133,7 +134,7 @@  void cpts_tx_timestamp(struct cpts *cpts, struct sk_buff *skb);
 int cpts_register(struct cpts *cpts);
 void cpts_unregister(struct cpts *cpts);
 struct cpts *cpts_create(struct device *dev, void __iomem *regs,
-			 u32 mult, u32 shift);
+			 struct device_node *node);
 void cpts_release(struct cpts *cpts);
 
 static inline void cpts_rx_enable(struct cpts *cpts, int enable)
@@ -170,7 +171,7 @@  static inline void cpts_tx_timestamp(struct cpts *cpts, struct sk_buff *skb)
 
 static inline
 struct cpts *cpts_create(struct device *dev, void __iomem *regs,
-			 u32 mult, u32 shift)
+			 struct device_node *node)
 {
 	return NULL;
 }