Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sdio driver #2

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mmahadevan108
Copy link

Initial SDIO Block device driver.
Add SDIO support for LPC55S69

@mmahadevan108
Copy link
Author

cc @0xc0170 @JojoS62 @maclobdell

@@ -0,0 +1,296 @@
/* mbed Microcontroller Library
* Copyright (c) 2017 ARM Limited
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

year 2019 and add also SPDX identifier please


int SDIOBlockDevice::program(const void *b, bd_addr_t addr, bd_size_t size)
{
//debug_if(SD_DBG, "program Card...\r\n");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unused code

@@ -0,0 +1,236 @@
/*
* Copyright (c) 2015, Freescale Semiconductor, Inc.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will these file work , if the location is target/fsl_sdmmc_common.h ? rather than TARGET_NXP as it's only for NXP targets?

Took the work done as part of PR
ARMmbed/mbed-os#8634

Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>
@mmahadevan108
Copy link
Author

I have update the PR to address the comments

//pin_mode(P0_17, PullNone);

/* SD Write Protect */
//pin_function(P0_15, 5);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about these unused code, lines 88-94 ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for catching this. This has been fixed.

Tested this with the block device test inside features-storage-tests-blockdevice-general_block_device
Below was the change made to add SDIO test

--- a/features/storage/TESTS/blockdevice/general_block_device/main.cpp
+++ b/features/storage/TESTS/blockdevice/general_block_device/main.cpp
@@ -47,6 +47,8 @@
 #include "FlashIAPBlockDevice.h"
 #endif

+#include "SDIOBlockDevice.h"
+
 using namespace utest::v1;

 #define TEST_BLOCK_COUNT 10
@@ -69,6 +71,7 @@ enum bd_type {
     dataflash,
     sd,
     flashiap,
+    sdio,
     default_bd
 };

@@ -90,6 +93,11 @@ static inline uint32_t align_up(uint32_t val, uint32_t size)
 static BlockDevice *get_bd_instance(uint8_t bd_type)
 {
     switch (bd_arr[bd_type]) {
+        case sdio: {
+            static SDIOBlockDevice default_bd(P0_17);
+            return &default_bd;
+            break;
+        }
         case spif: {
 #if COMPONENT_SPIF
             static SPIFBlockDevice default_bd(
@@ -632,6 +640,8 @@ void test_get_type_functionality()
     const char *bd_type = block_device->get_type();
     TEST_ASSERT_NOT_EQUAL(0, bd_type);

+    TEST_ASSERT_EQUAL(0, strcmp(bd_type, "SDIO"));
+
 #if COMPONENT_QSPIF
     TEST_ASSERT_EQUAL(0, strcmp(bd_type, "QSPIF"));
 #elif COMPONENT_SPIF
@@ -708,10 +718,12 @@ int get_bd_count()
     bd_arr[count++] = flashiap;       //4
 #endif

+    bd_arr[count++] = sdio;       //5
+
     return count;
 }

-static const char *prefix[] = {"SPIF ", "QSPIF ", "DATAFLASH ", "SD ", "FLASHIAP ", "DEFAULT "};
+static const char *prefix[] = {"SPIF ", "QSPIF ", "DATAFLASH ", "SD ", "FLASHIAP ", "SDIO ", "DEFAULT "};

 int main()
 {

Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>
@offirko
Copy link

offirko commented Feb 19, 2019

@mmahadevan108 : this PR#2 seems to be conflicting PR#1 on this same repo: (#1). For instance #1 is using DMA mode and #2 does not.

cc: @ARMmbed/mbed-os-storage

@mmahadevan108
Copy link
Author

mmahadevan108 commented Feb 19, 2019

I moved that part of the code to the target to decide the transfer mechanism to use.

@offirko
Copy link

offirko commented Feb 19, 2019

@mmahadevan108 - logically, leaving this part for the target to decide makes sense, but practically until someone changes PR #1 we have two separate PRs that conflict each other.

@0xc0170 - I think we need to decide whether we would like two separate SDIOBlockDevice repos, for each target.

@mmahadevan108
Copy link
Author

cc @flit

@mmahadevan108
Copy link
Author

How should we proceed?

@maclobdell
Copy link

maclobdell commented Mar 15, 2019

@mmahadevan108 - can you confirm usage details for testing this on LPC55S69_EVK? How did you test this? is card detect required? Or did you use something like this?

#include "SDIOBlockDevice.h"

BlockDevice *bd = new SDIOBlockDevice();

Also, there appears to be hard-coded pin seetings in sdio_device.c. Seems better to pass these in via the constructors.

/* SD POW_EN */
pin_function(P0_9, 2);
pin_mode(P0_9, PullNone);

@maclobdell
Copy link

maclobdell commented Mar 16, 2019

Never mind my last comment about usage details. I took the standard mbed-os-example-filesystem, made these changes and it works.

#include "BlockDevice.h"
#include "SDIOBlockDevice.h"

// This will take the system's default block device
//BlockDevice *bd = BlockDevice::get_default_instance();
BlockDevice *bd = new SDIOBlockDevice();
// Set up the button to trigger an erase
//InterruptIn irq(BUTTON1);
InterruptIn irq(SW2);

@mmahadevan108
Copy link
Author

Tested this with the block device test inside features-storage-tests-blockdevice-general_block_device
Below was the change made to add SDIO test

--- a/features/storage/TESTS/blockdevice/general_block_device/main.cpp
+++ b/features/storage/TESTS/blockdevice/general_block_device/main.cpp
@@ -47,6 +47,8 @@
 #include "FlashIAPBlockDevice.h"
 #endif

+#include "SDIOBlockDevice.h"
+
 using namespace utest::v1;

 #define TEST_BLOCK_COUNT 10
@@ -69,6 +71,7 @@ enum bd_type {
     dataflash,
     sd,
     flashiap,
+    sdio,
     default_bd
 };

@@ -90,6 +93,11 @@ static inline uint32_t align_up(uint32_t val, uint32_t size)
 static BlockDevice *get_bd_instance(uint8_t bd_type)
 {
     switch (bd_arr[bd_type]) {
+        case sdio: {
+            static SDIOBlockDevice default_bd(P0_17);
+            return &default_bd;
+            break;
+        }
         case spif: {
 #if COMPONENT_SPIF
             static SPIFBlockDevice default_bd(
@@ -632,6 +640,8 @@ void test_get_type_functionality()
     const char *bd_type = block_device->get_type();
     TEST_ASSERT_NOT_EQUAL(0, bd_type);

+    TEST_ASSERT_EQUAL(0, strcmp(bd_type, "SDIO"));
+
 #if COMPONENT_QSPIF
     TEST_ASSERT_EQUAL(0, strcmp(bd_type, "QSPIF"));
 #elif COMPONENT_SPIF
@@ -708,10 +718,12 @@ int get_bd_count()
     bd_arr[count++] = flashiap;       //4
 #endif

+    bd_arr[count++] = sdio;       //5
+
     return count;
 }

-static const char *prefix[] = {"SPIF ", "QSPIF ", "DATAFLASH ", "SD ", "FLASHIAP ", "DEFAULT "};
+static const char *prefix[] = {"SPIF ", "QSPIF ", "DATAFLASH ", "SD ", "FLASHIAP ", "SDIO ", "DEFAULT "};

 int main()
 {

mathias-arm added a commit to mathias-arm/sdio-driver that referenced this pull request Aug 9, 2019
- ARMmbed#1 is based on [JojoS62/sdio-driver/master](JojoS62@c03489c).
- ARMmbed#2 is based on [NXPmicro/sdio-driver/Add_SDIO_LPC55S69](nxp-archive@370c51a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants