Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

2014-05-29 Thread Satish Patel



On 5/29/2014 12:23 AM, Greg KH wrote:

On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:

SmartCard controller uses this interface to communicate with
SmartCard via PHY

Some SmartCard PHY has multiple slots for cards.
This inerface also enables controller to communicate
with one or more SmartCard connected over phy.

interface structure includes following APIs
- set/get config
- activate/deactivate smart card
- warm reset
- register_notify (for card insert/remove/overheat)
- unregister_notify

Signed-off-by: Satish Patel satish.pa...@ti.com
---
  Documentation/sc_phy.txt |  171 ++
  include/linux/sc_phy.h   |  136 
  2 files changed, 307 insertions(+)
  create mode 100644 Documentation/sc_phy.txt
  create mode 100644 include/linux/sc_phy.h


These are .h files, but where is the api functions that use
these structures defined at?

This is like template/wrappers, smart card phy driver will write API 
functions. And smartcard controller will call these functions.
With proposed approach, smartcard controller can communicate with any 
smart card phy (TI/NxP) without change in code. Using DT entry smartcard 
and PHY will gets connected with each other.

Refer diagram given @Documentation/sc_phy.txt.



confused,

greg k-h


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


Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

2014-05-29 Thread Satish Patel



On 5/29/2014 12:14 AM, Greg KH wrote:

On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:

+/**
+ * struct sc_phy - The basic smart card phy structure
+ *
+ * @dev: phy device
+ * @pdata: pointer to phy's private data structure
+ * @set_config: called to set phy's configuration
+ * @get_config: called to get phy's configuration
+ * @activate_card: perform smart card activation
+ * @deactivate_card: perform smart card de-activation
+ * @warm_reset: execute smart card warm reset sequence
+ * @register_card_activity_cb: register call back to phy device.
+ * This call back will be called on card insert or remove event
+ *
+ * smart card controller uses this interface to communicate with
+ * smart card via phy.Some smart card phy has multiple slots for
+ * cards. This inerface also enables controller to communicate with
+ * one or more smart card connected over phy.
+ */
+struct sc_phy {
+   /* phy's device pointer */
+   struct device *dev;


So this is the parent, right?  Why not embed a struct device into this
structure as well, further streaching out the device tree.


Do you mean to use dev-p for pdata ?
I have kept it outside, to give easeness/pragmatic focal for new phy 
driver development. Driver can make this pointer null and use above pointer.

+
+   /* phy's private data */
+   void *pdata;


If you do the above, then this pointer is not needed.



+
+   /* notify data, passed by interface user as a part of
+* register_notify API. Data should be passed back when
+* notification raised to the interface user
+*/
+   void *notify_data;


What makes this different from the pdata?
pdata is phy's private data, while notify_data is something phy will 
send to smart card controller on some event, like card 
remove/insert/timeout etc..




+
+   int (*set_config)(struct sc_phy *phy, u8 slot,
+   enum sc_phy_config attr, int value);
+   int (*get_config)(struct sc_phy *phy, u8 slot, enum
+   sc_phy_config attr);
+   int (*activate_card)(struct sc_phy *phy, u8 slot);
+   int (*deactivate_card)(struct sc_phy *phy, u8 slot);
+   int (*get_syncatr)(struct sc_phy *phy, u8 slot, u8 len, char *atr);
+   int (*warm_reset)(struct sc_phy *phy, u8 slot);
+   int (*register_notify)(struct sc_phy *phy,
+struct notifier_block *nb, void *notify_data);
+   int (*unregister_notify)(struct sc_phy *phy,
+   struct  notifier_block *nb);
+};
+
+#endif /* __SC_PHY_H__ */
--
1.7.9.5

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


Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

2014-05-29 Thread Rob Herring
On Thu, May 29, 2014 at 3:34 AM, Satish Patel satish.pa...@ti.com wrote:


 On 5/29/2014 12:23 AM, Greg KH wrote:

 On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:

 SmartCard controller uses this interface to communicate with
 SmartCard via PHY

 Some SmartCard PHY has multiple slots for cards.
 This inerface also enables controller to communicate
 with one or more SmartCard connected over phy.

 interface structure includes following APIs
 - set/get config
 - activate/deactivate smart card
 - warm reset
 - register_notify (for card insert/remove/overheat)
 - unregister_notify

 Signed-off-by: Satish Patel satish.pa...@ti.com
 ---
   Documentation/sc_phy.txt |  171
 ++
   include/linux/sc_phy.h   |  136 
   2 files changed, 307 insertions(+)
   create mode 100644 Documentation/sc_phy.txt
   create mode 100644 include/linux/sc_phy.h


 These are .h files, but where is the api functions that use
 these structures defined at?

 This is like template/wrappers, smart card phy driver will write API
 functions. And smartcard controller will call these functions.
 With proposed approach, smartcard controller can communicate with any smart
 card phy (TI/NxP) without change in code. Using DT entry smartcard and PHY
 will gets connected with each other.
 Refer diagram given @Documentation/sc_phy.txt.


 confused,

I believe the api Greg is wondering about is the notifier which as I
commented is not a good design.

There is now a phy subsystem. I don't know if it has what you need,
but you should look at it to determine if it will work or could be
extended to work.

Rob
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

2014-05-29 Thread Greg KH
On Thu, May 29, 2014 at 02:26:55PM +0530, Satish Patel wrote:
 
 
 On 5/29/2014 12:14 AM, Greg KH wrote:
 On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
 +/**
 + * struct sc_phy - The basic smart card phy structure
 + *
 + * @dev: phy device
 + * @pdata: pointer to phy's private data structure
 + * @set_config: called to set phy's configuration
 + * @get_config: called to get phy's configuration
 + * @activate_card: perform smart card activation
 + * @deactivate_card: perform smart card de-activation
 + * @warm_reset: execute smart card warm reset sequence
 + * @register_card_activity_cb: register call back to phy device.
 + * This call back will be called on card insert or remove event
 + *
 + * smart card controller uses this interface to communicate with
 + * smart card via phy.Some smart card phy has multiple slots for
 + * cards. This inerface also enables controller to communicate with
 + * one or more smart card connected over phy.
 + */
 +struct sc_phy {
 +   /* phy's device pointer */
 +   struct device *dev;
 
 So this is the parent, right?  Why not embed a struct device into this
 structure as well, further streaching out the device tree.
 
 Do you mean to use dev-p for pdata ?

No, use the device itself.

 I have kept it outside, to give easeness/pragmatic focal for new phy driver
 development. Driver can make this pointer null and use above pointer.

Ick, no, that's not how the driver model works.  Each device in the
system needs a struct device, don't try to chain off of an existing
device like this.  Make it a real one.

 +
 +   /* phy's private data */
 +   void *pdata;
 
 If you do the above, then this pointer is not needed.
 
 
 +
 +   /* notify data, passed by interface user as a part of
 +* register_notify API. Data should be passed back when
 +* notification raised to the interface user
 +*/
 +   void *notify_data;
 
 What makes this different from the pdata?
 pdata is phy's private data, while notify_data is something phy will send to
 smart card controller on some event, like card remove/insert/timeout etc..

That doesn't make much sense to me, why not just put that in the notify
function callback itself?

Please either use the existing phy layer of the kernel, or make a real
subsystem here, don't try to put a tiny shim on top of an existing
struct device for this driver, that's not how subsystems in Linux are
done.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

2014-05-29 Thread Greg KH
On Thu, May 29, 2014 at 08:47:31AM -0500, Rob Herring wrote:
 On Thu, May 29, 2014 at 3:34 AM, Satish Patel satish.pa...@ti.com wrote:
 
 
  On 5/29/2014 12:23 AM, Greg KH wrote:
 
  On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
 
  SmartCard controller uses this interface to communicate with
  SmartCard via PHY
 
  Some SmartCard PHY has multiple slots for cards.
  This inerface also enables controller to communicate
  with one or more SmartCard connected over phy.
 
  interface structure includes following APIs
  - set/get config
  - activate/deactivate smart card
  - warm reset
  - register_notify (for card insert/remove/overheat)
  - unregister_notify
 
  Signed-off-by: Satish Patel satish.pa...@ti.com
  ---
Documentation/sc_phy.txt |  171
  ++
include/linux/sc_phy.h   |  136 
2 files changed, 307 insertions(+)
create mode 100644 Documentation/sc_phy.txt
create mode 100644 include/linux/sc_phy.h
 
 
  These are .h files, but where is the api functions that use
  these structures defined at?
 
  This is like template/wrappers, smart card phy driver will write API
  functions. And smartcard controller will call these functions.
  With proposed approach, smartcard controller can communicate with any smart
  card phy (TI/NxP) without change in code. Using DT entry smartcard and PHY
  will gets connected with each other.
  Refer diagram given @Documentation/sc_phy.txt.
 
 
  confused,
 
 I believe the api Greg is wondering about is the notifier which as I
 commented is not a good design.

That, and the fact that if this really is an api, there are no .c
files for it like a normal api is in the kernel.

 There is now a phy subsystem. I don't know if it has what you need,
 but you should look at it to determine if it will work or could be
 extended to work.

I agree.  Satish, what's wrong with our existing phy layer?

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

2014-05-29 Thread Satish Patel



On 5/29/2014 7:17 PM, Rob Herring wrote:

On Thu, May 29, 2014 at 3:34 AM, Satish Patel satish.pa...@ti.com wrote:



On 5/29/2014 12:23 AM, Greg KH wrote:


On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:


SmartCard controller uses this interface to communicate with
SmartCard via PHY

Some SmartCard PHY has multiple slots for cards.
This inerface also enables controller to communicate
with one or more SmartCard connected over phy.

interface structure includes following APIs
- set/get config
- activate/deactivate smart card
- warm reset
- register_notify (for card insert/remove/overheat)
- unregister_notify

Signed-off-by: Satish Patel satish.pa...@ti.com
---
   Documentation/sc_phy.txt |  171
++
   include/linux/sc_phy.h   |  136 
   2 files changed, 307 insertions(+)
   create mode 100644 Documentation/sc_phy.txt
   create mode 100644 include/linux/sc_phy.h



These are .h files, but where is the api functions that use
these structures defined at?


This is like template/wrappers, smart card phy driver will write API
functions. And smartcard controller will call these functions.
With proposed approach, smartcard controller can communicate with any smart
card phy (TI/NxP) without change in code. Using DT entry smartcard and PHY
will gets connected with each other.
Refer diagram given @Documentation/sc_phy.txt.



confused,


I believe the api Greg is wondering about is the notifier which as I
commented is not a good design.

There is now a phy subsystem. I don't know if it has what you need,
but you should look at it to determine if it will work or could be
extended to work.

I have given my comments on notifier, it is required to notify real time 
events like card insert/remove to the smart card controller. As this 
interrupts tied to phy (in case phy is present) not with controller.


Existing phy subsystem does not support generic operations for the phy. 
If at all it adds supports for these, in future I am ok to get align 
with it.



Rob


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


Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

2014-05-29 Thread Satish Patel



On 5/29/2014 9:21 PM, Greg KH wrote:

On Thu, May 29, 2014 at 02:26:55PM +0530, Satish Patel wrote:



On 5/29/2014 12:14 AM, Greg KH wrote:

On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:

+/**
+ * struct sc_phy - The basic smart card phy structure
+ *
+ * @dev: phy device
+ * @pdata: pointer to phy's private data structure
+ * @set_config: called to set phy's configuration
+ * @get_config: called to get phy's configuration
+ * @activate_card: perform smart card activation
+ * @deactivate_card: perform smart card de-activation
+ * @warm_reset: execute smart card warm reset sequence
+ * @register_card_activity_cb: register call back to phy device.
+ * This call back will be called on card insert or remove event
+ *
+ * smart card controller uses this interface to communicate with
+ * smart card via phy.Some smart card phy has multiple slots for
+ * cards. This inerface also enables controller to communicate with
+ * one or more smart card connected over phy.
+ */
+struct sc_phy {
+   /* phy's device pointer */
+   struct device *dev;


So this is the parent, right?  Why not embed a struct device into this
structure as well, further streaching out the device tree.


Do you mean to use dev-p for pdata ?


No, use the device itself.


I have kept it outside, to give easeness/pragmatic focal for new phy driver
development. Driver can make this pointer null and use above pointer.


Ick, no, that's not how the driver model works.  Each device in the
system needs a struct device, don't try to chain off of an existing
device like this.  Make it a real one.


+
+   /* phy's private data */
+   void *pdata;


If you do the above, then this pointer is not needed.



+
+   /* notify data, passed by interface user as a part of
+* register_notify API. Data should be passed back when
+* notification raised to the interface user
+*/
+   void *notify_data;


What makes this different from the pdata?

pdata is phy's private data, while notify_data is something phy will send to
smart card controller on some event, like card remove/insert/timeout etc..


That doesn't make much sense to me, why not just put that in the notify
function callback itself?


Little correction over here..
Here is brief flow
USIM(Smart Card Controller) register notification callback to smarcard phy.

usim-phy-register_notify(phy handle, callback fn, notify_data);
notify_data : pass back when callback function will be called by PHY


Smartcard PHY - USIM
blocking_notifier_call_chain(notifier, action, notify_data);

action : card insert/remove/overheat etc..
notify data: USIM data passing back (supplied at the time of cb 
registration)




Please either use the existing phy layer of the kernel, or make a real
subsystem here, don't try to put a tiny shim on top of an existing
struct device for this driver, that's not how subsystems in Linux are
done.

Why I am not using exiting PHY framework ?

We will be more than happy to adapt generic phy if it
includes/add support for smart card requirements like

1) Some smart card phy (TDA8026-NxP) has multiple slots for smart
cards. This interface enables controller to communicate with specific
smart card inserted to the specific phy's slot.
2) Warm reset to smart card inserted to phy slot.
3) Bit banging of smart card pins to support vendor specific memory
cards.
4) Notification of card insert/remove/overheat etc.
5) synchronous and asynchronous modes for smart card transaction




thanks,

greg k-h


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


[PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

2014-05-28 Thread Satish Patel
SmartCard controller uses this interface to communicate with
SmartCard via PHY

Some SmartCard PHY has multiple slots for cards.
This inerface also enables controller to communicate
with one or more SmartCard connected over phy.

interface structure includes following APIs
- set/get config
- activate/deactivate smart card
- warm reset
- register_notify (for card insert/remove/overheat)
- unregister_notify

Signed-off-by: Satish Patel satish.pa...@ti.com
---
 Documentation/sc_phy.txt |  171 ++
 include/linux/sc_phy.h   |  136 
 2 files changed, 307 insertions(+)
 create mode 100644 Documentation/sc_phy.txt
 create mode 100644 include/linux/sc_phy.h

diff --git a/Documentation/sc_phy.txt b/Documentation/sc_phy.txt
new file mode 100644
index 000..d610e26
--- /dev/null
+++ b/Documentation/sc_phy.txt
@@ -0,0 +1,171 @@
+   SmartCard PHY Interface
+   Satish Patel satish.pa...@ti.com
+
+This document explains the SmartCard interface between SmartCard
+controller and SmartCard phy. Document also describes how-to-use.
+
+1. Why SmartCard PHY Interface?
+
+The use of smartcard is increasing in embedded industry. As smartcard
+not only prevent duplication but also, brings key based authentication
+flow into picture.
+
+SmartCard standards like EMV(EuroPay-Mastercard-Visa) are becoming
+mandatory for payment terminals.
+
+Till date, most of the SmartCard readers are based on USB serial
+interface. Which drives its logic within firmware lies on device.
+Few are based on FPGA solutions. But now SoCs are coming up with
+inbuilt smartcard controller. e.g. TI-AM43x
+
+Role of SmartCard controller and SmartCard phy:
+
+Smartcard Phy:
+Forms electrical interface between controller and SmartCard. Phy
+enables access to more than one smartcard and in addition it provides
+fast deactivation logic when card gets removed from the slot. It can
+also generate the signals like card insert/remove/overheat etc.
+
+Smartcard Controller:
+In built mechanism to meet EMV L1 specification (e.g. T=0, T=1
+protocol timings, ATR timeout etc..) for SmartCard transaction. In
+addition to this, it has FIFO to store tx/rx bytes, internal state
+machine for ATR, Tx/Rx, Synchronous/Asynchronous mode, timing
+counters etc..
+
+Controller can also have direct interface through which SmartCard
+can be connected without phy.
+
+Below is the brief of SmartCard block diagram from user to h/w
+layer.
+
+
+---
+|PC/SC App|
+---
+--- ---
+|PC/SC F/W| | Visa APP|
+--- ---
+--- 
+|IFD Hand.| | EMV L1/L2|| Test App |
+--- 
+User Space
+
+
+-
+|   SmartCard Controller Driver|
+-
+ | |
+ | |
+-  |
+| Phy Driver | |
+-  |
+ | |
+Kernel Space | |
+
+ | |
+   -  
+   | PHY   |  |Controller IP |
+   -  
+|   |
+
+|   |
+   ___
+   |||
+ VISA card   Master Card  Custom Card
+
+
+At present in Linux there is no public interface exist which acts as
+bridge between controller and phy. Mostly vendors uses proprietary
+solution in such cases.
+
+2. Introduction to SmartCard PHY interface
+
+SmartCard PHY interface that exposes phy's capabilities to the smart
+card controller. SmartCard controller uses this interface to
+communicate with SmartCard via phy.
+
+Such capabilities are:
+1) Some SmartCard phy (e.g. TDA8026-NxP) has multiple slots for smart
+cards. This interface enables controller to communicate with specific
+SmartCard inserted to the specific phy's slot.
+
+2) Warm reset to SmartCard inserted to phy slot.
+
+3) Bit banging of SmartCard pins to support vedor specific memory
+cards. Mostly when it comes to sychorous SmartCard
+
+4) Notification of card insert/remove/overheat etc.
+
+
+3. How to use
+
+SmartCard PHY:
+The SmartCard PHY driver, who wants to be interfaced with SmartCard
+controller require to follow below step
+
+- include sc_phy.h
+
+- use sc_phy structure as driver(client) data. PHY 

Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

2014-05-28 Thread Greg KH
On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
 +/**
 + * struct sc_phy - The basic smart card phy structure
 + *
 + * @dev: phy device
 + * @pdata: pointer to phy's private data structure
 + * @set_config: called to set phy's configuration
 + * @get_config: called to get phy's configuration
 + * @activate_card: perform smart card activation
 + * @deactivate_card: perform smart card de-activation
 + * @warm_reset: execute smart card warm reset sequence
 + * @register_card_activity_cb: register call back to phy device.
 + * This call back will be called on card insert or remove event
 + *
 + * smart card controller uses this interface to communicate with
 + * smart card via phy.Some smart card phy has multiple slots for
 + * cards. This inerface also enables controller to communicate with
 + * one or more smart card connected over phy.
 + */
 +struct sc_phy {
 + /* phy's device pointer */
 + struct device *dev;

So this is the parent, right?  Why not embed a struct device into this
structure as well, further streaching out the device tree.

 +
 + /* phy's private data */
 + void *pdata;

If you do the above, then this pointer is not needed.


 +
 + /* notify data, passed by interface user as a part of
 +  * register_notify API. Data should be passed back when
 +  * notification raised to the interface user
 +  */
 + void *notify_data;

What makes this different from the pdata?


 +
 + int (*set_config)(struct sc_phy *phy, u8 slot,
 + enum sc_phy_config attr, int value);
 + int (*get_config)(struct sc_phy *phy, u8 slot, enum
 + sc_phy_config attr);
 + int (*activate_card)(struct sc_phy *phy, u8 slot);
 + int (*deactivate_card)(struct sc_phy *phy, u8 slot);
 + int (*get_syncatr)(struct sc_phy *phy, u8 slot, u8 len, char *atr);
 + int (*warm_reset)(struct sc_phy *phy, u8 slot);
 + int (*register_notify)(struct sc_phy *phy,
 +  struct notifier_block *nb, void *notify_data);
 + int (*unregister_notify)(struct sc_phy *phy,
 + struct  notifier_block *nb);
 +};
 +
 +#endif /* __SC_PHY_H__ */
 -- 
 1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

2014-05-28 Thread Greg KH
On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
 SmartCard controller uses this interface to communicate with
 SmartCard via PHY
 
 Some SmartCard PHY has multiple slots for cards.
 This inerface also enables controller to communicate
 with one or more SmartCard connected over phy.
 
 interface structure includes following APIs
 - set/get config
 - activate/deactivate smart card
 - warm reset
 - register_notify (for card insert/remove/overheat)
 - unregister_notify
 
 Signed-off-by: Satish Patel satish.pa...@ti.com
 ---
  Documentation/sc_phy.txt |  171 
 ++
  include/linux/sc_phy.h   |  136 
  2 files changed, 307 insertions(+)
  create mode 100644 Documentation/sc_phy.txt
  create mode 100644 include/linux/sc_phy.h

These are .h files, but where is the api functions that use
these structures defined at?

confused,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html