Re: [PATCH v5] can: xilinx CAN controller support.

[SÃren Brinkmann]

On Wed, 2014-03-12 at 11:18AM +0100, Michal Simek wrote:
> 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@xxxxxxxxxx)
> >> 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.

Okay, then 'bus_clk' would probably be fine. That is hopefully obvious
enough to be mapped to a clock input of that IP, while being generic
enough to allow other buses as well.

	SÃren


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/