[PATCH v9 0/8] Generic PHY Framework

From: Kishon Vijay Abraham I <hidden>
Date: 2013-07-08 12:17:21
Also in: linux-devicetree, linux-omap

Hi,

On Monday 08 July 2013 04:54 PM, Patel, Satish wrote:

Hi,

quoted

-----Original Message-----
From: Balbi, Felipe
Sent: Thursday, July 04, 2013 3:42 PM
To: ABRAHAM, KISHON VIJAY
Cc: Patel, Satish; Balbi, Felipe; grant.likely at linaro.org;
tony at atomide.com; arnd at arndb.de; swarren at nvidia.com;
sylvester.nawrocki at gmail.com; linux-kernel at vger.kernel.org; linux-
omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
usb at vger.kernel.org; gregkh at linuxfoundation.org; akpm at linux-
foundation.org; rob.herring at calxeda.com; rob at landley.net;
linux at arm.linux.org.uk; benoit.cousson at linaro.org; mchehab at redhat.com;
cesarb at cesarb.net; davem at davemloft.net; Nayak, Rajendra;
shawn.guo at linaro.org; Shilimkar, Santosh; devicetree-
discuss at lists.ozlabs.org; linux-doc at vger.kernel.org; Nori, Sekhar;
Krishnamoorthy, Balaji T; Cherian, George; Mankad, Maulik Ojas
Subject: Re: [PATCH v9 0/8] Generic PHY Framework

On Thu, Jul 04, 2013 at 03:25:32PM +0530, Kishon Vijay Abraham I
wrote:

quoted

On Thursday 04 July 2013 02:51 PM, Patel, Satish wrote:

quoted

Hi,

quoted

-----Original Message-----
From: Balbi, Felipe
Sent: Wednesday, July 03, 2013 6:51 PM
To: ABRAHAM, KISHON VIJAY
Cc: Patel, Satish; grant.likely at linaro.org; tony at atomide.com;

Balbi,

quoted

Felipe; arnd at arndb.de; swarren at nvidia.com;
sylvester.nawrocki at gmail.com; linux-kernel at vger.kernel.org; linux-
omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
usb at vger.kernel.org; gregkh at linuxfoundation.org; akpm at linux-
foundation.org; rob.herring at calxeda.com; rob at landley.net;
linux at arm.linux.org.uk; benoit.cousson at linaro.org;

mchehab at redhat.com;

quoted

cesarb at cesarb.net; davem at davemloft.net; Nayak, Rajendra;
shawn.guo at linaro.org; Shilimkar, Santosh; devicetree-
discuss at lists.ozlabs.org; linux-doc at vger.kernel.org; Nori, Sekhar;
Krishnamoorthy, Balaji T; Cherian, George
Subject: Re: [PATCH v9 0/8] Generic PHY Framework

Hi,

On Wed, Jul 03, 2013 at 03:35:39PM +0530, Kishon Vijay Abraham I
wrote:

quoted

On Wednesday 03 July 2013 03:02 PM, Patel, Satish wrote:

quoted

Hi Kishon,

quoted

-----Original Message-----
From: ABRAHAM, KISHON VIJAY
Sent: Wednesday, June 26, 2013 5:17 PM
To: grant.likely at linaro.org; tony at atomide.com; Balbi, Felipe;

ABRAHAM,

quoted

KISHON VIJAY; arnd at arndb.de; swarren at nvidia.com;
sylvester.nawrocki at gmail.com; linux-kernel at vger.kernel.org;

linux-

quoted

quoted
quoted
quoted
quoted
quoted
omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org;

linux-

quoted

usb at vger.kernel.org; gregkh at linuxfoundation.org; akpm at linux-
foundation.org
Cc: rob.herring at calxeda.com; rob at landley.net;

linux at arm.linux.org.uk;

quoted

benoit.cousson at linaro.org; mchehab at redhat.com;

cesarb at cesarb.net;

quoted

davem at davemloft.net; Nayak, Rajendra; shawn.guo at linaro.org;

Shilimkar,

quoted

Santosh; devicetree-discuss at lists.ozlabs.org; linux-
doc at vger.kernel.org; Nori, Sekhar; Krishnamoorthy, Balaji T;

Cherian,

quoted

George
Subject: [PATCH v9 0/8] Generic PHY Framework

Added a generic PHY framework that provides a set of APIs for

the

quoted

PHY

quoted

drivers
to create/destroy a PHY and APIs for the PHY users to obtain a
reference to
the PHY with or without using phandle.

This framework will be of use only to devices that uses

external

quoted

PHY

quoted

(PHY
functionality is not embedded within the controller).

The intention of creating this framework is to bring the phy

drivers

quoted

spread
all over the Linux kernel to drivers/phy to increase code re-

use

quoted

and

quoted

to
increase code maintainability.

I would like to use this framework for a smart-card controller

connected to a

quoted

smart-card phy. I have some questions and would like to get

feedback on the same.

quoted

glad to know that :-)

quoted

I am using ?TDA8026" Smartcard PHY from NXP. Here is the link

for

quoted

datasheet

quoted

and app note for the same. The smart card controller is inside

the

quoted

TI SoC

quoted

I am working with.

Datasheet :
www.nxp.com/documents/data_sheet/TDA8026.pdf?

Appnote :
http://www.nxp.com/documents/application_note/AN10724.pdf

The TI SoC details are not public (yet). I can provide details

to

quoted

you offline.

quoted

Brief about operation:
-	The controller can work with and without a PHY
-	When not using PHY, it is limited to talking to a single
	smart card. There is also a need to put external de-activation

logic

quoted

	on card removal for this case.
-	With a PHY you can use more than one smart card.
-	Phy has 5 slots :  1 for smart card (credit/debit/other

card

quoted

with chip)

quoted

       and others for SAM ? SIM like modules
- 	Once the PHY is initialized, there are some operations

that the

quoted

controller

quoted

	can request of the PHY like:
	- Card configurations  - set voltage
	- Activation of card
	- ATR ? Answer to reset
	- Warm reset
	- ADPU exchange
	- Deactivation ( Normal/Emergency)

hmm.. We should think about extending the phy_ops to include

these

quoted

operations (something like phy_smart_card_ops so that other
smart_card PHYs will also be able to use it).

let's try to avoid use-case specific additions. set_voltage sounds
like
a regulator thing, but the regulator is controlled through the

PHY. I

quoted

guess it makes sense to have a generic phy_set_voltage() call

since

quoted

even
USB can make use of that.

For card activation, it sounds like phy_init()/phy_shutdown()

would

quoted

cover it.

For warm reset perhaps a phy_reset() callback ? Although that

could,

quoted

easily, get abused.

For deactivation, that's phy_shutdown().

ATR and ADPU needs more thought, I guess.

quoted

- 	In the mode when smartcard controller talks directly to

the

quoted

card without the need

quoted

	for a PHY, all the above operations will be carried out by the

controller itself

quoted

My current thought process is to make the controller driver

provide

quoted

the user interface

quoted

and talk to the PHY using the generic PHY framework you

proposed.

quoted

In the case where there

quoted

is no PHY, my idea is to create a "dummy" PHY which uses the

controller functionality itself.

quoted

right. And in the case where you actually have a PHY, create a

PHY

quoted

driver and implement the phy_smart_card_ops and register with the

PHY

quoted

framework.

I would try to avoid that. Otherwise we will have phy_usb_ops,
phy_sata_ops, phy_network_ops, phy_pci_ops, etc etc etc. It would
easily
blow up.

- I do agree with you. Creating Phy specific ops will blow up whole
   concept of generic phy f/w.
- Can we have interface like phy_setconfig - with parameter like
   phy_setconfig(int param, void *value)
	- Here param can be enum of available config parameters for
	  specific phy.
Phy can perform different operation/set internal state based on
param selection and value passed by.

e.x in case of smartcard
	enum set_config {
		SET_VOLATAGE,
		SET_ACTIVATE,
		SET_WARMRESET,
		SET_ATR,
		SET_DEACTIVE,
		....
	};

hmm.. this looks similar to ioctl and can be abused easily IMO :s

+1

What we need is to come up with generic ways to model those, if we
need
set voltage, then add a phy_set_voltage() kinda thing, perhaps add
capability flags to enable/disable support fort those (just like mmc
does).

But adding something which can handle "anything" like
phy_set_config(),
it's the same as adding use-case specific ops.

We have two options over here

Option 1:

Defining generic api to which can be mapped over multiple phys
For smartcard case, I have can thought of following mapping with new
generic apis as suggested.

Smartcard_poweron -> power_on
Smartcard_powerdown -> power_off
Smartcard_set_voltage -> phy_set_voltage
Smartcard_activate_card -> phy_activate_slot
Smartcard_deactivate_card -> phy_deactivate_slot

how is activate/deactivate different from poweron/poweroff in this
use case?

Smartcard_set_c4/c8/rst/io -> phy_set_pin

Whats should be exactly done here? Looks to me like it should be
part of init. Does these pin settings need to be changed dynamically?

Smartcard_warm_reset -> phy_warmreset

Again looks to me like it should be part of init.

Smartcard_set_clkdiv -> phy_set_clock
Smartcard_get_clkdiv -> phy_get_clock

Why would the smartcard give clock to the PHY? Shouldn't the driver
of the PHY driver be doing that by itself?

Smartcard_set_atr_mute_timeout -> ??
Smartcard_set_atr_early_timeout -> ??	
Smartcard_get_isr_status -> phy_get_status

Why? Is it like the detection of an external event? Then the PHY
driver should use *extcon* for event detection and passing.

Smartcard_get_version -> phy_get_version

Again why? Why would the smartcard need the version of the PHY?

Thanks
Kishon

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help