linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: monstr@monstr.eu (Michal Simek)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5] can: xilinx CAN controller support.
Date: Wed, 12 Mar 2014 11:18:51 +0100	[thread overview]
Message-ID: <5320348B.7030401@monstr.eu> (raw)
In-Reply-To: <531F1E30.4040203@pengutronix.de>

Hi guys,


On 03/11/2014 03:31 PM, Marc Kleine-Budde wrote:
> On 03/11/2014 03:08 PM, Appana Durga Kedareswara Rao wrote:
> 
>>>>>> +   struct napi_struct napi;
>>>>>> +   u32 (*read_reg)(const struct xcan_priv *priv, enum xcan_reg reg);
>>>>>> +   void (*write_reg)(const struct xcan_priv *priv, enum xcan_reg reg,
>>>>>> +                   u32 val);
>>>>>> +   struct net_device *dev;
>>>>>> +   void __iomem *reg_base;
>>>>>> +   unsigned long irq_flags;
>>>>>> +   struct clk *aperclk;
>>>>>> +   struct clk *devclk;
>>>>>
>>>>> Please rename the clock variables to match the names in the DT.
>>>>>
>>>> The clock names are different for axi CAN and CANPS case.
>>>> So will make them as busclk and devclk Are you ok with this?
>>>
>>> Why not "ref_clk" and "aper_clk" as used in the DT?
>>>
>> One of the comments I got from the Soren(sorenb at xilinx.com)
>> Is the  clock-names must match the data sheet.
>> If I Modify the clock names then it is different names for AXI CAN
>> and CANPS case.
> 
> Sorry, my faul, I thought the names are already these from the
> datasheet. As S?ren pointed out please use 's_axi_aclk' and
> 'can_clk' for the DT and for the the variable names in the private
> struct, too.
> 
> The 'official' name of the ip core seems to be axi_can, should we rename
> the driver? I suspect, that Michal wants to keep xilinx in the name for
> marketing reasons :P

I hope that I am not moving to marketing position.  :-)

opb_can, plb_can, axi_can, amba_can are all valid options for this IP.

Maybe in future Xilinx will decide to use different bus and then will just move
all current soft IPs to new bus and drivers will be compatible.
This is exactly what happened when Xilinx moved from OPB to PLB and then
from PLB to AXI.
That's why I think in general having bus name in name doesn't fit for our case.

The same is for clock name which has bus name in it.
For PLB it was called SPLB_Clk and I don't have OPB version but
at least standalone driver points to OPB version where I believe
SPLB_Clk name was not used.

That's why if you want to reflect that clock is coming from bus
you should use any generic name. At least for these soft IPs
and this one is special because it is one which also went to silicon.
Next example is XADC.

That's why I just don't think we can find out better name than
xilinx_can.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140312/87c9427e/attachment-0001.sig>

  reply	other threads:[~2014-03-12 10:18 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-04 13:20 [PATCH v5] can: xilinx CAN controller support Kedareswara rao Appana
2014-03-04 23:51 ` Sören Brinkmann
2014-03-05  6:58   ` Oliver Hartkopp
2014-03-05  7:04     ` Appana Durga Kedareswara Rao
     [not found] ` <20140304235115.GN13293@xsjandreislx>
2014-03-05  7:03   ` Appana Durga Kedareswara Rao
2014-03-10 14:57 ` Marc Kleine-Budde
2014-03-10 15:04   ` Michal Simek
2014-03-10 15:10     ` Marc Kleine-Budde
2014-03-10 15:15       ` Arnd Bergmann
2014-03-10 15:26         ` Michal Simek
2014-03-11  4:45           ` Fengguang Wu
2014-03-11  9:38             ` Michal Simek
2014-03-13 11:21               ` Fengguang Wu
2014-03-11 12:34   ` Appana Durga Kedareswara Rao
2014-03-11 12:48     ` Marc Kleine-Budde
2014-03-11 14:08       ` Appana Durga Kedareswara Rao
2014-03-11 14:31         ` Marc Kleine-Budde
2014-03-12 10:18           ` Michal Simek [this message]
2014-03-12 16:18             ` Sören Brinkmann
2014-03-12 10:01       ` Michal Simek
2014-03-20  4:41         ` Appana Durga Kedareswara Rao
2014-03-10 15:15 ` Rob Herring
2014-03-10 15:22   ` Michal Simek
     [not found]   ` <531DD8C3.5050006@xilinx.com>
2014-03-11 12:35     ` Appana Durga Kedareswara Rao
     [not found] <1393939253-30245-1-git-send-email-appanad@xilinx.com>
2014-03-10  7:12 ` Appana Durga Kedareswara Rao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5320348B.7030401@monstr.eu \
    --to=monstr@monstr.eu \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).